feat(risk): add PR risk assessment scoring to review pipeline by maruiz93 · Pull Request #861 · fullsend-ai/agents

maruiz93 · 2026-08-18T08:10:49Z

Summary

Add composite PR risk score (1–5) computed as a synchronous pre-pass sub-agent in the review pipeline
Score combines three signal tiers: deterministic metadata (50%), git history analysis (30%), and linked issue context (20%)
Results surfaced via risk/* labels (with traffic-light colors) and a sticky PR comment
Score is purely informational — does not gate the review outcome
Gated by REVIEW_RISK_ASSESSMENT_ENABLED env var (default true in harness)

Changes

Component	Files	Description
Tier 1 script	`skills/pr-risk-assessment/scripts/risk-tier1.sh`	Deterministic metadata signals (blast radius, path sensitivity, CI impact, dependencies, test ratio, author context)
Scoring skill	`skills/pr-risk-assessment/SKILL.md`	Three-tier scoring model with weights, anchoring examples, output format
Sub-agent	`skills/pr-review/sub-agents/risk-assessment.md`	Sonnet-based pre-pass sub-agent definition
Orchestrator	`skills/pr-review/SKILL.md`	Roster, generalized skill-loading table (replaces docs-currency special case), step 3c-2
Schema	`schemas/review-result.schema.json`	Optional `risk_assessment` field with `risk_signal` def
Post-review	`scripts/post-review.sh`	Risk label application + sticky comment
Feature flag	`harness/review.yaml`, `scripts/pre-review.src.sh`	`REVIEW_RISK_ASSESSMENT_ENABLED` in harness sandbox env; `REVIEW_GIT_FETCH_DEPTH` auto-defaults to `"0"` when enabled
Unit tests	`scripts/risk-tier1-test.sh`	30 tests for signal computation functions
Integration tests	`scripts/post-review-test.sh`	6 new risk label/comment test cases
Eval cases	`eval/review/cases/001-risk-low-typo-fix/`, `002-risk-high-auth-change/`	Functional eval fixtures
Docs	`docs/review.md`	Risk labels table, `REVIEW_RISK_ASSESSMENT_ENABLED` and `REVIEW_GIT_FETCH_DEPTH` variables

Deviations from issue #4698

Informational only: Score does not gate review outcome, auto-merge eligibility, or model selection (deferred to later phases)
Sub-agent placement: Tier 1 script runs inside the sandbox sub-agent (not pre-review.sh); aligns with the orchestrator's skill-dispatch model
Level taxonomy: low/moderate/elevated/high/critical (5 levels) vs issue's low/medium/high/critical (4 levels)
Security patterns: Uses directory-based patterns from security-triage.md (e.g., auth/, rbac/, token/); .pem/.key extension patterns deferred

Part of fullsend-ai/fullsend#4698

Test plan

bash scripts/risk-tier1-test.sh — 30 unit tests pass
bash scripts/post-review-test.sh — 63 tests pass (6 new risk tests)
bash scripts/validate-output-schema-test.sh — 46 schema tests pass
Run review eval cases (001-risk-low-typo-fix, 002-risk-high-auth-change)
Manual test: trigger review on a test PR with risk assessment enabled

🤖 Generated with Claude Code

fullsend-ai-review · 2026-08-18T08:12:10Z

🤖 Review · ⚠️ Cancelled · Started 8:12 AM UTC · Ended 8:18 AM UTC

Commit: c848b26 · View workflow run →

qodo-code-review · 2026-08-18T08:12:25Z

PR Summary by Qodo

Add composite PR risk scoring to the review pipeline

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Compute optional 1–5 PR risk scores from metadata, history, and linked issues.
• Persist assessments without changing approval or change-request outcomes.
• Publish color-coded risk labels and sticky comments with comprehensive test coverage.

Diagram

sequenceDiagram
    participant O as Review Orchestrator
    participant A as Risk Agent
    participant T as Tier 1 Script
    participant G as Git History
    participant I as Linked Issue
    participant J as Result JSON
    participant P as Post Review
    actor R as GitHub PR
    O->>A: Spawn when enabled
    A->>T: Compute metadata
    T->>R: Fetch PR metadata
    A->>G: Analyze file history
    A->>I: Evaluate issue context
    A-->>O: Return risk object
    O->>J: Store assessment
    J->>P: Provide review result
    P->>R: Apply label and comment

High-Level Assessment

The following are alternative approaches to this PR:

1. Deterministic core with issue-context agent

➕ Produces reproducible Tier 1, Tier 2, weighting, and rounding behavior
➕ Reduces model cost and scoring variance
➕ Retains semantic analysis where linked issue interpretation is necessary
➖ Requires substantially more executable git-history and scoring logic
➖ Needs broader unit coverage for history edge cases
➖ Still requires orchestration between deterministic and model-generated signals

2. Inline scoring in the review orchestrator

➕ Avoids an additional synchronous sub-agent invocation
➕ Keeps all review result construction in one execution context
➖ Further expands an already complex orchestration prompt
➖ Couples risk scoring changes to the core review workflow
➖ Makes independent testing and graceful degradation harder

Recommendation: Prefer a deterministic scoring core for metadata, git history, weighting, and score mapping, with the sub-agent limited to linked-issue interpretation and rationale generation. The PR's modular, feature-gated pre-pass is a sound initial boundary, but moving objective calculations into executable code would make risk labels more stable and auditable before broad reliance.

Files changed (16) +1035 / -8

Enhancement (6) +605 / -8

review-result.schema.jsonAdd optional risk assessment result schema +34/-0

Add optional risk assessment result schema
• Defines the score, categorical level, rationale, and optional tier signal arrays. Signal entries are constrained to non-empty dimension and value strings.
schemas/review-result.schema.json

post-review.shPublish risk labels and sticky comments +65/-0

Publish risk labels and sticky comments

• Validates the risk level, removes stale risk labels, creates and applies the color-coded current label, and posts a sanitized sticky assessment comment. Risk presentation remains independent of the review disposition.

scripts/post-review.sh

SKILL.mdIntegrate risk scoring into review orchestration +78/-8

Integrate risk scoring into review orchestration

• Registers risk assessment as a feature-gated synchronous pre-pass, defines its context and failure behavior, and stores successful output in the final review result. It also generalizes linked-skill loading beyond the previous documentation-specific case.

skills/pr-review/SKILL.md

risk-assessment.mdDefine the risk assessment sub-agent +71/-0

Define the risk assessment sub-agent

• Introduces a read-only Sonnet sub-agent that gathers metadata and history signals, evaluates linked issues, and returns only a raw JSON risk object. It explicitly excludes code review, findings, and verdict ownership.

skills/pr-review/sub-agents/risk-assessment.md

SKILL.mdSpecify the three-tier risk scoring model +206/-0

Specify the three-tier risk scoring model

• Documents weighted metadata, git-history, and linked-issue tiers with score thresholds, missing-context redistribution, anchoring examples, graceful degradation, and the required JSON contract.

skills/pr-risk-assessment/SKILL.md

risk-tier1.shExtract deterministic Tier 1 risk signals +151/-0

Extract deterministic Tier 1 risk signals

• Fetches PR files and author metadata to calculate blast radius, sensitive paths, CI and dependency impact, test ratio, bot status, and contributor history. Individual retrieval failures degrade affected outputs to 'UNKNOWN'.

skills/pr-risk-assessment/scripts/risk-tier1.sh

Tests (9) +429 / -0

annotations.yamlDefine low-risk typo evaluation expectations +16/-0

Define low-risk typo evaluation expectations
• Requires the low-risk label and forbids high or critical labels for the documentation-only fixture. It also sets evaluation cost and turn limits.
eval/review/cases/001-risk-low-typo-fix/annotations.yaml

input.yamlCreate low-risk pull request fixture +17/-0

Create low-risk pull request fixture
• Defines a single-file README typo correction intended to receive a score of one and a 'risk/low' label.
eval/review/cases/001-risk-low-typo-fix/input.yaml

README.mdSeed typo fixture repository +7/-0

Seed typo fixture repository
• Provides the misspelled README baseline used by the low-risk evaluation case.
eval/review/cases/001-risk-low-typo-fix/repo/README.md

annotations.yamlDefine high-risk authentication evaluation expectations +15/-0

Define high-risk authentication evaluation expectations
• Requires the authentication refactor to avoid a low-risk classification and expects a high or critical score. It also calls for protected-path detection of the CODEOWNERS change.
eval/review/cases/002-risk-high-auth-change/annotations.yaml

input.yamlCreate high-risk authentication pull request fixture +66/-0

Create high-risk authentication pull request fixture
• Defines a multi-file authentication and RBAC refactor that changes security-sensitive code and CODEOWNERS while linking an issue.
eval/review/cases/002-risk-high-auth-change/input.yaml

README.mdSeed authentication fixture documentation +3/-0

Seed authentication fixture documentation
• Provides minimal repository context for the high-risk authentication evaluation case.
eval/review/cases/002-risk-high-auth-change/repo/README.md

handler.goSeed original authentication handler +15/-0

Seed original authentication handler
• Provides the static-token authentication baseline that the high-risk fixture refactors into provider-based validation.
eval/review/cases/002-risk-high-auth-change/repo/internal/auth/handler.go

post-review-test.shTest risk labels and comments after review +54/-0

Test risk labels and comments after review

• Extends the GitHub CLI mock for risk label operations and verifies high, low, and elevated labels, sticky comments, logging, absent assessments, and request-change outcomes.

scripts/post-review-test.sh

risk-tier1-test.shAdd deterministic signal unit tests +236/-0

Add deterministic signal unit tests

• Adds 30 shell tests covering blast radius, protected and security-sensitive paths, CI and dependency detection, test-file ratios, and bot authorship. The test helpers mirror the production signal functions.

scripts/risk-tier1-test.sh

Other (1) +1 / -0

review.envEnable risk assessment by default +1/-0

Enable risk assessment by default

• Exports 'FULLSEND_RISK_ASSESSMENT_ENABLED' with a default value of 'true', allowing deployments to disable the pre-pass explicitly.

env/review.env

fullsend-ai-review · 2026-08-18T08:19:18Z

🤖 Finished Review · ✅ Success · Started 8:19 AM UTC · Completed 8:41 AM UTC

Commit: d2f4860 · View workflow run →

qodo-code-review · 2026-08-18T08:21:09Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

1. Empty flag enables assessment 📜 Skill insight ≡ Correctness

Description

The orchestrator documentation says an empty FULLSEND_RISK_ASSESSMENT_ENABLED disables the
pre-pass, but ${FULLSEND_RISK_ASSESSMENT_ENABLED:-true} converts both unset and empty values to
true. As a result, deployments using an explicitly empty value cannot disable the feature as
documented, and the guard does not trigger under one of its stated conditions.

Code

env/review.env[8]

+export FULLSEND_RISK_ASSESSMENT_ENABLED="${FULLSEND_RISK_ASSESSMENT_ENABLED:-true}"

Relevance

●●● Strong
Explicit empty-value semantics are a documented contract; similar empty-fallback mismatch findings
were accepted.
PR-#567

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
${VAR:-true} substitutes true when the variable is either unset or empty, while the review
procedure's consumer contract explicitly states that an empty value skips risk assessment. These
producer and consumer semantics contradict each other and cannot both hold at runtime.
env/review.env[8-8]
skills/pr-review/SKILL.md[542-546]
skills/pr-review/SKILL.md[540-546]
Skill: code-review
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An explicitly empty risk-assessment feature flag is converted to `true`, contradicting the review skill's documented behavior that an empty value disables or skips the pre-pass.

## Issue Context
Choose one behavior and make the environment expansion, documentation, and tests consistent. If an empty value must disable risk assessment, use shell defaulting that distinguishes an unset variable from an empty one; otherwise, update the documented contract so that only `false` disables the feature.

## Fix Focus Areas
- env/review.env[8-8]
- skills/pr-review/SKILL.md[540-546]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Protected paths require human approval~~ ✗ Dismissed 📜 Skill insight § Compliance

Description

This PR modifies protected scripts/ and skills/ governance paths. Although linked issue #4698
provides justification, these changes still require human approval and must not be auto-approved.

Code

scripts/post-review.sh[R445-448]

+# ---------------------------------------------------------------------------
+# Risk assessment: apply risk/* label and post breakdown comment.
+# Risk level is informational only — it does not gate the review outcome.
+# Label logic is mirrored in post-review-test.sh — update both.

Relevance

●●● Strong
Protected governance-path changes historically require explicit review attention despite
linked-issue justification.
PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist explicitly classifies scripts/ and skills/ as protected paths and requires a
finding whenever they are modified. This PR adds review-pipeline behavior under both path groups;
the linked issue lowers the justification concern but does not remove the human-approval
requirement.
scripts/post-review.sh[445-448]
skills/pr-review/SKILL.md[540-546]
skills/pr-risk-assessment/scripts/risk-tier1.sh[1-9]
Skill: pr-review

3. RISK_LEVEL remains partially sanitized ✓ Resolved 📜 Skill insight ⛨ Security

Description

The workflow warning interpolates an invalid RISK_LEVEL after removing only raw newlines and
carriage returns. It remains unsanitized for ::, encoded newlines, ANSI escapes, and other control
characters, allowing untrusted result data to alter GitHub Actions log commands.

Code

scripts/post-review.sh[462]

+      echo "::warning::Invalid risk level '${RISK_LEVEL}', skipping risk label"

Relevance

●●● Strong
Repo strongly and repeatedly accepts hardening of untrusted values interpolated into GitHub Actions
workflow commands.
PR-#573
PR-#757

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist requires complete sanitization of every interpolated GitHub Actions workflow-command
value. The new code removes only raw LF and CR at lines 456–457, then directly interpolates
RISK_LEVEL into ::warning:: at line 462.

scripts/post-review.sh[455-462]
Skill: code-review
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The invalid-risk warning interpolates `RISK_LEVEL` into a GitHub Actions workflow command after only partial sanitization.

## Issue Context
Every interpolated workflow-command value must independently remove or encode `::`, `%0A`, `%0D`, ANSI escapes, and control characters before emission.

## Fix Focus Areas
- scripts/post-review.sh[455-462]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View high (3)

4. ~~Line counts multiply files~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The jq expression independently iterates every addition and deletion, producing a Cartesian product
rather than summing each file's own additions and deletions. For a multi-file PR this multiplies the
true total by the file count, inflating both LINES_CHANGED and BLAST_RADIUS.

Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[51]
+LINES_CHANGED=$(echo "${PR_FILES_JSON}" | jq '[.[].additions + .[].deletions] | add // 0')

Relevance

●●● Strong
jq expression structurally multiplies additions/deletions independently, an objective aggregation
bug affecting scoring.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The API result is parsed as an array of file objects, but line 51 independently expands that array
twice. The inflated result is immediately passed to blast-radius classification and is also a
separately scored Tier 1 signal.
skills/pr-risk-assessment/scripts/risk-tier1.sh[48-63]
skills/pr-risk-assessment/SKILL.md[46-55]
scripts/risk-tier1-test.sh[34-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`LINES_CHANGED` uses two independent `.[]` jq iterators, which cross-multiplies additions and deletions across files.

## Issue Context
Compute additions plus deletions within each file object before summing the resulting array. Add a production-script test with multiple files and asymmetric counts.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[48-54]
- scripts/risk-tier1-test.sh[30-57]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Changed files stop at 100~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Tier 1 script requests per_page=100 without pagination, so every signal ignores changed files
after the first page. Large PRs therefore report at most 100 files and can miss protected,
security-sensitive, CI, dependency, and test changes entirely.

Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[32]
+PR_FILES_JSON=$(gh api "repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}/files?per_page=100" 2>/dev/null) || PR_FILES_JSON=""

Relevance

●●● Strong
Single-page/uncapped API fetch truncation risk closely matches an accepted pagination-related fix.
PR-#708

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script makes one request capped at 100 entries, and every subsequent Tier 1 metric is derived
from that single response. The scoring model explicitly includes file-count ranges above 50 and
signals that may occur in omitted files.
skills/pr-risk-assessment/scripts/risk-tier1.sh[31-51]
skills/pr-risk-assessment/scripts/risk-tier1.sh[65-126]
skills/pr-risk-assessment/SKILL.md[46-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Changed-file retrieval requests only the first 100 files, causing incomplete Tier 1 signals for larger PRs.

## Issue Context
Use `gh api --paginate` and combine returned pages into one JSON array before computing any signals. Add coverage with more than 100 mocked files and a sensitive file on a later page.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[31-51]
- scripts/risk-tier1-test.sh[30-207]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Risk result omitted~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The orchestrator stores risk_assessment, but the authoritative pipeline output contract in
agents/review.md omits it and instructs the agent to include only listed fields. An agent
following that higher-priority contract can discard a successfully computed score, so no risk label
or comment is produced.

Code

skills/pr-review/SKILL.md[R591-592]

+6. Store the `risk_assessment` object for inclusion in
+   `agent-result.json` (step 7).

Relevance

●● Moderate
Plausible contract conflict but no close historical precedent to confirm reviewer action.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changed skill requires retaining the object, while the authoritative agent definition says only
its listed fields may be emitted and omits risk_assessment; the review skill explicitly states
that the agent definition wins conflicts.

skills/pr-review/SKILL.md[587-592]
agents/review.md[228-249]
skills/pr-review/SKILL.md[1325-1330]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The risk pre-pass result can be omitted because the authoritative review-agent output contract does not list `risk_assessment` or include it in JSON construction examples.

## Issue Context
The PR review skill explicitly says the agent definition wins when instructions conflict. Update the authoritative field table and output construction guidance so a computed risk object is carried into every applicable result action.

## Fix Focus Areas
- agents/review.md[228-249]
- agents/review.md[273-317]
- skills/pr-review/SKILL.md[1276-1280]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~Security paths match substrings~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Security-sensitive patterns are matched anywhere in a path rather than as path components, so
directories such as unauth/ or notokens/ match auth/ or tokens/. These false positives
inflate the security-sensitive count and composite risk score.

Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[R81-82]
+    if [[ "${file}" == *"${pattern}"* ]]; then
+      security_count=$((security_count + 1))

Relevance

●●● Strong
Accepted precedent supports correcting path-classification false positives and robust pattern
matching in repository scripts.
PR-#94

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Every security pattern is directory-shaped, but the matcher wraps it in unrestricted wildcards. The
resulting count directly maps to elevated Tier 1 sub-scores.
skills/pr-risk-assessment/scripts/risk-tier1.sh[24-29]
skills/pr-risk-assessment/scripts/risk-tier1.sh[77-87]
skills/pr-risk-assessment/SKILL.md[52-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Security directory names are recognized as arbitrary substrings, producing matches inside unrelated directory names.

## Issue Context
Match slash-delimited path components rather than arbitrary text while retaining support for nested security directories. Add negative tests such as `internal/unauth/file.go` and `vendor/notokens/file.go`.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[24-29]
- skills/pr-risk-assessment/scripts/risk-tier1.sh[77-87]
- scripts/risk-tier1-test.sh[95-126]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Stale risk survives fallback~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

When risk assessment is disabled or its sub-agent fails, risk_assessment is absent and the entire
post-review block is skipped. Any risk label and sticky comment from a prior review remain visible,
presenting an obsolete score as current.

Code

scripts/post-review.sh[R450-451]

+HAS_RISK=$(jq 'has("risk_assessment")' "${RESULT_FILE}")
+if [[ "${HAS_RISK}" == "true" ]]; then

Relevance

●●● Strong
Stale state/label cleanup omissions are accepted when repeated runs can leave inconsistent state.
PR-#567

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The skill explicitly permits omission on disablement or pre-pass failure, while post-review performs
all stale-label removal and sticky-comment updates only inside the HAS_RISK == true branch. The
no-risk test merely checks that no new risk command occurs and does not model stale state.
skills/pr-review/SKILL.md[542-546]
skills/pr-review/SKILL.md[594-599]
scripts/post-review.sh[450-508]
scripts/post-review-test.sh[978-983]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An absent risk result leaves labels and the sticky comment from earlier review runs unchanged.

## Issue Context
Absence is an expected state when the feature is disabled or the pre-pass fails. Ensure this state removes prior risk artifacts, or explicitly replaces the comment with an unavailable status.

## Fix Focus Areas
- scripts/post-review.sh[445-508]
- scripts/post-review-test.sh[978-983]
- skills/pr-review/SKILL.md[542-546]
- skills/pr-review/SKILL.md[594-599]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. ~~Score-level mismatch accepted~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

The schema validates score and level independently and accepts contradictory pairs such as score
5 with level low. Post-review then applies the label from level while displaying the conflicting
numeric score, producing an internally inconsistent assessment.

Code

schemas/review-result.schema.json[R34-37]

+        "score": { "type": "integer", "minimum": 1, "maximum": 5 },
+        "level": {
+          "type": "string",
+          "enum": ["low", "moderate", "elevated", "high", "critical"]

Relevance

●●● Strong
Schema permits contradictory score/level pairs despite an explicit documented mapping, causing
inconsistent output.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The sub-agent defines an exact score-to-level mapping, but the schema only checks membership in
separate numeric and string ranges. The post-review script independently reads both values and
chooses the label solely from level.
schemas/review-result.schema.json[30-53]
skills/pr-review/sub-agents/risk-assessment.md[60-64]
scripts/post-review.sh[452-489]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Schema validation permits score and level values that contradict the documented one-to-one mapping.

## Issue Context
Use conditional schema constraints to enforce 1=low, 2=moderate, 3=elevated, 4=high, and 5=critical. Add valid and invalid pair tests.

## Fix Focus Areas
- schemas/review-result.schema.json[30-53]
- scripts/validate-output-schema-test.sh[413-441]
- skills/pr-review/sub-agents/risk-assessment.md[60-64]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View medium (3)

10. ~~Nested manifests go undetected~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Dependency matching only recognizes manifests at the repository root, so common paths such as
frontend/package.json or services/api/go.mod produce DEPENDENCY_FILES_CHANGED=none. This
understates risk for dependency changes in monorepos and nested projects.

Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[R101-104]

+  case "${file}" in
+    go.mod|go.sum|package.json|package-lock.json|yarn.lock|\
+    requirements.txt|requirements*.txt|Pipfile|Pipfile.lock|\
+    Gemfile|Gemfile.lock|pom.xml|build.gradle|Cargo.toml|Cargo.lock)

Relevance

●●● Strong
Nested dependency paths are a deterministic coverage bug; accepted history favors fixing scripts
that silently miss relevant repository data.
PR-#94

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The shell case contains only bare manifest names and no path-prefix alternatives, while the signal
is documented generally as changed dependency files. Existing tests exercise only root-level
examples.
skills/pr-risk-assessment/scripts/risk-tier1.sh[98-112]
scripts/risk-tier1-test.sh[149-176]
skills/pr-risk-assessment/SKILL.md[54-54]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Dependency manifests are detected only when their complete path equals a root-level filename.

## Issue Context
Match recognized manifest basenames at any directory depth while preserving the full changed path in output. Add nested-manifest regression cases.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[98-112]
- scripts/risk-tier1-test.sh[149-176]
- skills/pr-risk-assessment/SKILL.md[54-54]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. ~~High-risk eval allows no label~~ ✓ Resolved 📜 Skill insight ▣ Testability

Description

The high-risk evaluation requires no label and merely forbids risk/low, despite stating that
risk/high or risk/critical must be applied. The case can therefore pass with no risk label or
with an incorrect moderate/elevated label.

Code

eval/review/cases/002-risk-high-auth-change/annotations.yaml[R3-6]
+labels:
+  required: []
+  forbidden:
+    - risk/low

Relevance

●●● Strong
Fixture expectation explicitly requires high/critical label, but annotation allows no label or wrong
label.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The annotation has an empty required label list, while its own review expectation says a high or
critical label should be applied. Forbidding only risk/low does not constrain that behavior.
eval/review/cases/002-risk-high-auth-change/annotations.yaml[3-14]
Skill: code-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The high-risk evaluation does not mechanically constrain the expected `risk/high` or `risk/critical` outcome.

## Issue Context
Use the evaluation framework's supported alternative-label assertion, or add equivalent cases/assertions that fail unless one of the two expected labels is present.

## Fix Focus Areas
- eval/review/cases/002-risk-high-auth-change/annotations.yaml[3-14]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

12. ~~Protected filenames match prefixes~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Protected entries representing exact files are checked as prefixes, so names such as
Dockerfile.backup, CODEOWNERS.old, or CLAUDE.md.disabled are counted as protected. This can
incorrectly raise PROTECTED_PATH_COUNT from its baseline score to 3 or 5.

Code

skills/pr-risk-assessment/scripts/risk-tier1.sh[R69-70]
+    if [[ "${file}" == "${pattern}"* ]]; then
+      protected_count=$((protected_count + 1))

Relevance

●●● Strong
Accepted shell correctness fixes are common; exact-file prefix matching is a trivial deterministic
false positive.
PR-#94

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The protected list mixes directory prefixes with exact filenames, but the matcher appends * to
every entry. The scoring model materially raises risk as soon as one such match is counted.
skills/pr-risk-assessment/scripts/risk-tier1.sh[16-22]
skills/pr-risk-assessment/scripts/risk-tier1.sh[65-75]
skills/pr-risk-assessment/SKILL.md[51-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Exact protected filenames are matched with prefix semantics, classifying unrelated similarly named files as protected.

## Issue Context
Use exact equality for file entries and prefix matching only for directory entries ending in `/`. Add negative tests for filename suffixes.

## Fix Focus Areas
- skills/pr-risk-assessment/scripts/risk-tier1.sh[16-22]
- skills/pr-risk-assessment/scripts/risk-tier1.sh[65-75]
- scripts/risk-tier1-test.sh[60-93]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

13. ~~Tier1 tests duplicate implementation~~ ✓ Resolved 📜 Skill insight ▣ Testability

Description

The new test suite reimplements production signal functions instead of invoking risk-tier1.sh, so
it can pass while the production script behaves differently. For example, the production
LINES_CHANGED aggregation is not exercised anywhere in the test suite.

Code

scripts/risk-tier1-test.sh[R34-37]

+classify_blast_radius() {
+  local files="$1"
+  local lines="$2"
+  if [ "${files}" -lt 5 ] && [ "${lines}" -lt 100 ]; then

Relevance

● Weak
Closely matching test-drift findings about reimplementing production logic in tests were explicitly
rejected multiple times.
PR-#488
PR-#708
PR-#508

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test defines its own classify_blast_radius implementation and similarly duplicates the other
signal functions. The production script separately computes API-derived values, including
LINES_CHANGED, but no test invokes that code path.
scripts/risk-tier1-test.sh[30-44]
skills/pr-risk-assessment/scripts/risk-tier1.sh[31-54]
Skill: code-implementation

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Tier 1 tests execute copied implementations rather than the production script, leaving the changed production behavior unconstrained.

## Issue Context
Mock `gh api`, run `skills/pr-risk-assessment/scripts/risk-tier1.sh`, and assert its actual `KEY=VALUE` output, including multi-file line aggregation and API failure fallbacks.

## Fix Focus Areas
- scripts/risk-tier1-test.sh[34-44]
- skills/pr-risk-assessment/scripts/risk-tier1.sh[31-54]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

code-review

code-implementation

pr-review

docs-review

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

fullsend-ai-review · 2026-08-18T08:40:53Z

Review

Findings

Medium

[protected-path] This PR modifies 13 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/post-review-test.sh — Several risk-assessment test cases use truncated names with ellipsis ("risk-a...", "risk-l...", "risk-i..." ×4). The four "risk-i..." instances share the same name, so later tests overwrite the run directory and stdout log of earlier ones. Each test checks immediately before the next runs, so test results are correct, but post-mortem diagnosis of a failed test is harder because only the last test's artifacts survive.
Remediation: Give each test a unique, descriptive kebab-case name.
[test-integrity] scripts/validate-output-schema-test.sh — Eight risk-assessment schema tests share the truncated name "risk-s...", and one each share "risk-m..." and "risk-a...". These collide in the test directory, so each overwrites the prior fixture. Test results are correct because each assertion runs synchronously, but shared names make post-mortem debugging ambiguous.
Remediation: Give each test a unique, descriptive name.
[credential-in-process-list] scripts/pre-review.sh — The git fetch URL embeds GH_TOKEN as a command-line argument. While stderr is redirected to /dev/null, the token is visible in the process list for the duration of the fetch. This is the standard pattern used by actions/checkout and across this codebase, so practical risk is limited to same-runner process enumeration.
[error-handling-idiom] scripts/pre-review.sh — The clone-deepening block uses base64 -w0 (GNU-specific). The pre-script runs in a controlled Linux CI environment so practical impact is nil, but other scripts in the repo use openssl base64 -e -A for portability.

Previous run

Review

Findings

Medium

[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/post-review-test.sh — Several risk-assessment test cases use truncated names with ellipsis ("risk-a...", "risk-l...", "risk-i..." ×4). The four "risk-i..." instances share the same name, so later tests overwrite the run directory and stdout log of earlier ones. Each test checks immediately before the next runs, so test results are correct, but post-mortem diagnosis of a failed test is harder because only the last test's artifacts survive.
Remediation: Give each test a unique, descriptive kebab-case name.
[credential-in-process-list] scripts/pre-review.sh — The git fetch URL embeds GH_TOKEN as a command-line argument. While stderr is redirected to /dev/null, the token is visible in the process list for the duration of the fetch. This is the standard pattern used by actions/checkout and across this codebase, so practical risk is limited to same-runner process enumeration.

Previous run (2)

Review

Findings

Medium

[logic-error] harness/review.yaml — REVIEW_RISK_ASSESSMENT_ENABLED is added to env.sandbox but not to env.runner. The pre-script (scripts/pre-review.sh) runs on the runner and checks ${REVIEW_RISK_ASSESSMENT_ENABLED:-false} to gate clone deepening for git history analysis. Since the variable is absent from env.runner, the default-false guard prevents clone deepening from ever triggering, leaving Tier 2 risk signals (git history) permanently degraded in pipeline runs.
Remediation: Add REVIEW_RISK_ASSESSMENT_ENABLED: "true" to the env.runner section of harness/review.yaml.
[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/post-review-test.sh — Multiple risk-assessment test cases use truncated names with ellipsis ("risk-a...", "risk-l...", "risk-i..." ×4). The four "risk-i..." instances share the same name, causing later tests to silently overwrite run directory and stdout artifacts of earlier ones, making test failure diagnosis unreliable.
Remediation: Give each test a unique, descriptive kebab-case name.
[test-adequacy] scripts/validate-output-schema-test.sh — No schema validation tests for the new risk_assessment object's allOf constraints (five if/then blocks enforcing score-to-level consistency).
Remediation: Add test cases for valid and mismatched risk_assessment score/level.
[credential-in-process-list] scripts/pre-review.sh — The git fetch URL embeds GH_TOKEN as a command-line argument. While stderr is redirected to /dev/null, the token is visible in the process list for the duration of the fetch. This is the standard pattern used by actions/checkout and across this codebase, so practical risk is limited to same-runner process enumeration.
[stale-reference] docs/review.md — The paragraph below the Variables table says "Override either variable" but the table now lists six configurable variables after this PR added two new rows.
Remediation: Change "Override either variable" to "Override these variables".

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (3)

Review

Findings

Medium

[logic-error] harness/review.yaml — REVIEW_RISK_ASSESSMENT_ENABLED is added to env.sandbox but not to env.runner. The pre-script (scripts/pre-review.sh) runs on the runner and checks this variable to gate clone deepening for git history analysis. Since the variable is absent from env.runner, the default-false guard ${REVIEW_RISK_ASSESSMENT_ENABLED:-false} prevents clone deepening from ever triggering, leaving Tier 2 risk signals (git history) permanently degraded in pipeline runs.
Remediation: Add REVIEW_RISK_ASSESSMENT_ENABLED: "true" to the env.runner section of harness/review.yaml.
[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[credential-in-process-list] scripts/pre-review.sh:502 — The git fetch URL embeds GH_TOKEN as a command-line argument. While stderr is redirected to /dev/null, the token is visible in the process list for the duration of the fetch. This is the standard pattern used by actions/checkout, so practical risk is limited to same-runner process enumeration.
Remediation: Use git credential helpers or GIT_ASKPASS. Alternatively, accept the risk given alignment with actions/checkout precedent.
[test-integrity] scripts/post-review-test.sh:1704 — Multiple risk-assessment test cases use truncated names with ellipsis ("risk-a...", "risk-l...", "risk-i..." ×3), breaking the descriptive kebab-case naming convention used elsewhere. The three "risk-i..." instances share the same name, causing later tests to silently overwrite run directory and stdout artifacts of earlier ones, making test failure diagnosis unreliable. See also: [naming-convention] finding at this location.
Remediation: Give each test a unique, descriptive kebab-case name.
[test-adequacy] scripts/validate-output-schema-test.sh — No schema validation tests for the new risk_assessment object's allOf constraints (five if/then blocks enforcing score-to-level consistency).
Remediation: Add test cases for valid and mismatched risk_assessment score/level.
[GHA-sanitization-gap] scripts/post-review.sh:837 — The ::warning::Invalid risk level message uses inline sanitization (strip % and :) consistent with the nearest precedent but differs from the _gha_sanitize helper used elsewhere. The case statement restricts RISK_LEVEL to five known values before this point, limiting impact to the * fallback branch with an already-sanitized value.
Remediation: Use _gha_sanitize for consistency.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (4)

Review

Findings

Medium

[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/post-review-test.sh:1704 — Three distinct test cases reuse the name "risk-i..." (lines 1704, 1708, 1715, 1719). Because the test name is embedded in the run directory path (${TMPDIR}/run-${test_name}) and stdout log path, colliding names cause later tests to silently overwrite artifacts of earlier ones. While tests currently pass correctly due to sequential execution, the FAIL output prints only FAIL: risk-i... with no way to distinguish which scenario failed. See also: [naming-convention] finding below.
Remediation: Give each test a unique, descriptive name.
[naming-convention] scripts/post-review-test.sh:1679 — Six risk-assessment test names are truncated with ellipsis ("risk-a...", "risk-l...", three instances of "risk-i..."), breaking the descriptive kebab-case naming convention used by every other test in this file (e.g., "approve-no-downgrade", "label-actions-applied"). See also: [test-integrity] finding above.
Remediation: Use descriptive names matching the existing convention.
[test-adequacy] scripts/validate-output-schema-test.sh — No schema validation tests for the new risk_assessment object's allOf constraints (five if/then blocks enforcing score-to-level consistency).
Remediation: Add test cases for valid and mismatched risk_assessment score/level.
[GHA-sanitization-gap] scripts/post-review.sh:837 — The ::warning::Invalid risk level message uses inline sanitization (strip % and :) consistent with the nearest precedent (lines 525–528 for REVIEW_FINDING_SEVERITY_THRESHOLD) but differs from the _gha_sanitize helper used elsewhere. The case statement restricts RISK_LEVEL to five known values before this point, limiting impact to the * fallback branch with an already-sanitized value.
Remediation: Use _gha_sanitize for consistency.

Previous run (5)

Review

Findings

Medium

[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[error-handling] skills/pr-risk-assessment/scripts/risk-tier1.sh:149 — When gh api returns a non-array JSON error response with exit code 0, jq -s 'add' produces an object rather than an array. The -z guard catches empty strings but not non-array JSON. Most API errors are caught by || PR_FILES_JSON="", limiting this to edge cases.
Remediation: Add jq -e 'type == "array"' validation after the -z guard.
[test-integrity] scripts/risk-tier1-test.sh:166 — The pagination test's gh stub uses return in a standalone script (heredoc written to a file and chmod +x'd). In bash, return outside a function or sourced script emits a warning to stderr and is a no-op; the case ;; terminator correctly ends the branch regardless, so the test passes for the right reason.
Remediation: Replace return with exit 0 to suppress the spurious stderr warning.
[test-adequacy] scripts/validate-output-schema-test.sh — No schema validation tests for the new risk_assessment object's allOf constraints (five if/then blocks enforcing score-to-level consistency).
Remediation: Add test cases for valid and mismatched risk_assessment score/level.
[GHA-sanitization-gap] scripts/post-review.sh:825 — The ::warning::Invalid risk level message uses inline sanitization (strip % and :) consistent with the nearest precedent (lines 525–528 for REVIEW_FINDING_SEVERITY_THRESHOLD) but differs from the _gha_sanitize helper used elsewhere. The case statement on line 818 restricts RISK_LEVEL to five known values before this point, limiting impact to the * fallback branch with an already-sanitized value.
Remediation: Use _gha_sanitize for consistency.
[code-organization] scripts/post-review.sh:831 — The risk-label removal loop (for stale_risk in risk/low risk/moderate ...) appears three times in the new risk assessment block in post-review.src.sh (and identically in the generated post-review.sh). A helper would consolidate the list of valid risk levels to a single definition.
Remediation: Extract a remove_stale_risk_labels() helper.

Previous run (6)

Review

Findings

Medium

[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[error-handling] skills/pr-risk-assessment/scripts/risk-tier1.sh:149 — When gh api returns a non-array JSON error response with exit code 0, jq -s 'add' produces an object rather than an array. The -z guard catches empty strings but not non-array JSON. Most API errors are caught by || PR_FILES_JSON="", limiting this to edge cases.
Remediation: Add jq -e 'type == "array"' validation after the -z guard.
[test-integrity] scripts/risk-tier1-test.sh:166 — The pagination test's gh stub uses return instead of exit 0 in a standalone script context. The test passes because output is already echoed before return fails.
Remediation: Replace return with exit 0.
[test-adequacy] scripts/validate-output-schema-test.sh — No schema validation tests for the new risk_assessment object's allOf constraints (five if/then blocks enforcing score-to-level consistency).
Remediation: Add test cases for valid and mismatched risk_assessment score/level.
[GHA-sanitization-gap] scripts/post-review.sh:569 — The ::warning::Invalid risk level message uses inline sanitization rather than the _gha_sanitize helper used elsewhere, missing ANSI escape stripping. Impact is minimal — the value is immediately discarded.
Remediation: Use _gha_sanitize for consistency.
[code-organization] scripts/post-review.sh:879 — The risk-label removal loop is repeated three times in post-review.sh and identically in post-review.src.sh. A helper function would reduce duplication.
Remediation: Extract a remove_all_risk_labels() helper.

Previous run (7)

Review

Findings

Medium

[protected-path] This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and the description explains the rationale. Human approval is always required for protected-path changes, regardless of context.
[missing-documentation] docs/review.md — The new REVIEW_GIT_FETCH_DEPTH environment variable is introduced in this PR (added to harness/review.yaml env.runner, consumed by pre-review.sh) but is not documented in the docs/review.md Variables table. The PR adds REVIEW_RISK_ASSESSMENT_ENABLED to this table but omits the companion variable.
Remediation: Add a row to the Variables table for REVIEW_GIT_FETCH_DEPTH.

Low

[error-handling] skills/pr-risk-assessment/scripts/risk-tier1.sh — When gh api returns a non-array JSON error response with exit code 0, jq -s 'add' produces an object rather than an array. The subsequent jq -r '.[].filename' fails silently, producing FILES_CHANGED=0 and misleadingly specific signals instead of the intended UNKNOWN fallback.
Remediation: Add a jq -e 'type == "array"' validation after the -z guard.
[test-integrity] scripts/risk-tier1-test.sh — The pagination test's gh stub uses return instead of exit 0 in a standalone script context. return in a non-sourced script is a bash error; the test passes because jq ignores the extra output.
Remediation: Replace return with exit 0 in the pagination test stub.
[provenance-warning] Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run (8)

Review

Findings

Medium

[protected-path] agents/review.md — This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/): agents/review.md, harness/review.yaml, scripts/post-review-test.sh, scripts/post-review.sh, scripts/post-review.src.sh, scripts/pre-review.sh, scripts/pre-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh:147 — When gh api --paginate returns an empty JSON array, the script produces FILES_CHANGED=0 instead of UNKNOWN. All downstream functions handle zero arguments correctly, so no actual bug, but the behavioral distinction (0 vs UNKNOWN) is worth noting for debugging.
[error-handling] skills/pr-risk-assessment/scripts/risk-tier1.sh:186 — The gh api call uses --jq to project the response into {author, assoc}, then the result is parsed by two separate jq -r invocations for .author and .assoc. This double jq-parse is redundant — consider extracting both fields in a single pass.
[test-adequacy] scripts/risk-tier1-test.sh:154 — The e2e test stub returns a single JSON array, not exercising the multi-page pagination path of gh api --paginate. The jq -s 'add' merge in risk-tier1.sh is untested for the multi-page case.
[scope-creep] skills/pr-risk-assessment/SKILL.md — The linked issue (Add PR-level risk assessment score to the review pipeline fullsend#4698) requests gating capabilities (model selection, sub-agent count, auto-merge eligibility) that this PR deliberately defers. The scope narrowing is documented in the PR body, but no follow-up issue tracks the deferred work.
[tool-list-pattern-deviation] skills/pr-review/sub-agents/risk-assessment.md:7 — This sub-agent includes Bash in its tool list (Read, Bash, Grep, Glob), unlike all other sub-agents in this directory which use Read, Grep, Glob. The deviation is justified (the sub-agent runs risk-tier1.sh and git log commands) but breaks the established pattern.
[env-var-documentation] docs/review.md — REVIEW_GIT_FETCH_DEPTH is added to harness/review.yaml env.runner but is not documented in the env var reference table in docs/review.md. Downstream harness composers who use base: composition may need to discover it from the harness YAML source.

Previous run (9)

Review

Findings

Medium

[protected-path] agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review.src.sh, scripts/pre-review.sh, scripts/pre-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh — This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and provides clear rationale for the changes. Human approval is always required for protected-path changes, regardless of context.

Low

[schema-enforcement-gap] schemas/review-result.schema.json:30 — The risk_assessment schema documents that score and level must be consistent (1=low, 2=moderate, 3=elevated, 4=high, 5=critical) but enforces no structural constraint between them. A mismatched pair would produce cosmetically odd output in the sticky comment, though the label remains correct since it is derived from level. The field is informational only, limiting operational impact.
Remediation: Add allOf entries mapping each score const to its corresponding level const.
[naming-convention] skills/pr-review/sub-agents/risk-assessment.md:6 — The model frontmatter uses claude-sonnet-4-6@default. While similar to security-triage as a pre-pass, this sub-agent performs more complex work (Tier 2 git history analysis, Tier 3 issue context evaluation, weighted scoring) that justifies sonnet over haiku.
[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh:119 — In find_dependency_files, the case pattern lists requirements.txt as a literal match followed by requirements*.txt as a glob. The glob already covers the literal, making the first entry redundant.
[comment-style] scripts/post-review.sh:666 — The is_control_label function's new risk/* block adds a # Pipeline-managed label prefixes comment where existing entries use no inline comments. The comment explains a different matching mechanism (glob vs array iteration), which justifies the annotation.
[env-var-placement] harness/review.yaml:55 — REVIEW_GIT_FETCH_DEPTH is in env.runner only, REVIEW_RISK_ASSESSMENT_ENABLED is in env.sandbox only. Both placements are correct — each variable is consumed by only one side (pre-script vs sandbox agent).

Previous run (10)

Review

Findings

Medium

[protected-path] agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review.src.sh, scripts/pre-review.sh, scripts/pre-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh — 12 of 21 changed files are under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and describes the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.
[missing-documentation] docs/review.md — New env var REVIEW_GIT_FETCH_DEPTH is added to harness/review.yaml env.runner but is not documented in the Variables table in docs/review.md, where all other REVIEW_* env vars are listed. Users overriding the harness via base: composition would not discover this knob.
Remediation: Add a row to the Variables table.

Low

[score-level-consistency] schemas/review-result.schema.json — The schema description states score and level must be consistent (1=low, 2=moderate, etc.) but no JSON Schema constraint enforces this invariant. Post-review.sh also validates them independently without cross-checking. A sub-agent could return {"score": 5, "level": "low"} and both validations would pass. Practical impact is limited since the sub-agent is instructed to maintain consistency.
Remediation: Consider adding if/then constraints in the schema or deriving the level from the score programmatically in post-review.sh.
[comment-style] scripts/post-review.sh — Inline comment references "same pattern as lines 108-116" which is fragile to future edits. Since post-review.sh is generated from post-review.src.sh, line numbers will shift across edits to either file.
Remediation: Reference the sanitization pattern by description rather than line number.

Previous run (11)

Review

Findings

Medium

[protected-path] agents/review.md — This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/): agents/review.md, harness/review.yaml, scripts/post-review-test.sh, scripts/post-review.sh, scripts/post-review.src.sh, scripts/pre-review.sh, scripts/pre-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and provides rationale for the changes. Human approval is always required for protected-path changes, regardless of context.
[drift-risk] skills/pr-risk-assessment/scripts/risk-tier1.sh:21 — The PROTECTED_PATHS hardcoded fallback array in risk-tier1.sh is a third copy of the protected paths list (alongside harness/review.yaml env.runner/env.sandbox and post-review-test.sh). The existing drift test in post-review-test.sh only checks the harness YAML against the test file — it does not check the risk-tier1.sh fallback. If someone updates the protected paths in the harness and post-review-test.sh but misses risk-tier1.sh, the fallback would silently use stale values when REVIEW_PROTECTED_PATHS is unset.
Remediation: Add a drift check in risk-tier1-test.sh that extracts the PROTECTED_PATHS array from risk-tier1.sh and compares it against harness/review.yaml.
[missing-documentation] docs/review.md — The new REVIEW_GIT_FETCH_DEPTH env var is added to harness/review.yaml (env.runner) and consumed by scripts/pre-review.sh to deepen shallow clones for Tier 2 risk signals, but it is not documented in the Variables table in docs/review.md. All other REVIEW_* env vars introduced or existing (REVIEW_FINDING_SEVERITY_THRESHOLD, REVIEW_SKIP_AUTHORS, REVIEW_PROTECTED_PATHS, REVIEW_RISK_ASSESSMENT_ENABLED) are documented there.
Remediation: Add a row for REVIEW_GIT_FETCH_DEPTH to the Variables table in docs/review.md.

Low

[score-level-consistency] schemas/review-result.schema.json:29 — The schema description states score and level must be consistent (1=low, 2=moderate, 3=elevated, 4=high, 5=critical) and that the post-script validates both independently. However, the post-script validates score range (1-5 regex) and level enum (case statement) separately without cross-validating their consistency. A sub-agent could return {"score": 1, "level": "critical"} and it would pass all validation.
[gitlab-coverage] scripts/pre-review.sh:470 — The clone-deepening logic only handles FULLSEND_FORGE=github (using GH_TOKEN for authenticated fetch). For GitLab, the else branch emits a warning but does not attempt to deepen. Tier 2 risk signals will be degraded on GitLab if the clone is shallow. The SKILL.md documents graceful degradation for shallow repos, so this is by design, but worth noting for future GitLab support.

Previous run (12)

Review

Findings

Medium

[missing-doc] docs/review.md — REVIEW_GIT_FETCH_DEPTH is added to harness/review.yaml env.runner but is not documented in the docs/review.md Variables table. Users wanting to customize clone deepening behavior have no reference.
Remediation: Add a row to the Variables table for REVIEW_GIT_FETCH_DEPTH.
[protected-path] harness/review.yaml, agents/review.md, scripts/*, skills/* — This PR modifies 12 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and provides clear rationale for the changes. Human approval is required for protected-path changes regardless of context.

Low

[arithmetic-accuracy] skills/pr-risk-assessment/SKILL.md — The "Security fix in crypto module" anchoring example states the composite is 3.8, but 0.50×3.0 + 0.30×4.5 + 0.20×4.5 = 3.75. The final rounded result (4) is still correct.
Remediation: Change 3.8 to 3.75 in the anchoring table.
[scope-gap] skills/pr-review/SKILL.md — The linked issue scope includes review effort calibration and auto-merge gating, but the PR implements the score as purely informational. If intentionally deferred, document this as deferred scope.
[scope-creep] harness/review.yaml — REVIEW_RISK_ASSESSMENT_ENABLED defaults to true (opt-out). Per AGENTS.md §8, this is a valid static default. Consider whether opt-in rollout is more appropriate for a new feature.
[naming-convention] skills/pr-risk-assessment/scripts/risk-tier1.sh — Hardcoded fallback PROTECTED_PATHS array can drift from the harness default. The fallback is unreachable in production but has no drift-detection test.
Remediation: Add a comment marking as test-only default, or add a drift test.
[stale-reference] docs/review.md — "Override either variable" paragraph is stale; the table now has 5+ entries. Pre-existing issue made worse by this PR.
Remediation: Change "Override either variable" to "Override these variables".
[scope-gap] harness/review.yaml — REVIEW_RISK_ASSESSMENT_ENABLED docs do not clarify it only governs sandbox-side dispatch; the post-script checks result JSON presence independently.

Previous run (13)

Review

Findings

Medium

[protected-path] agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh — This PR modifies 10 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and explains the rationale for each component. Human approval is always required for protected-path changes.
[logic-error] schemas/review-result.schema.json:270 — The risk_assessment schema description states "Score and level must be consistent: 1=low, 2=moderate, 3=elevated, 4=high, 5=critical. Post-script validates both independently." The post-script validates score range (1–5) and level enum separately but does not cross-check mutual consistency. A result with {score: 1, level: "critical"} passes validation, applies a risk/critical label, and posts a comment reading "Risk Assessment: critical (1/5)." Consider adding a score-to-level lookup table in the post-script to warn on mismatches, or correcting the schema description.
[workflow-command-injection] scripts/post-review.src.sh:485 — RISK_SCORE is interpolated into a GHA ::warning:: workflow command without sanitization. Compare to RISK_LEVEL, which receives newline/percent/colon stripping before any ::warning:: interpolation. The schema validation layer (integer 1–5) provides a primary barrier, but the post-script should be independently safe (defense-in-depth). The same issue exists in the bundled scripts/post-review.sh. Consider applying the same sanitization pattern used for RISK_LEVEL before the ::warning:: echo.

Low

[edge-case] skills/pr-review/SKILL.md:709 — The skill-loading table in step 4 (Part 3) includes risk-assessment, but step 4 explicitly excludes risk-assessment from its dispatch loop. Step 3c-2 cross-references this table, creating organizational ambiguity. The existing pre-pass sub-agent (security-triage) avoids this by having no linked skill entry. Consider moving the entry inline into step 3c-2 or adding a note clarifying the table is shared.
[missing-test] scripts/post-review-test.sh — The risk test suite covers invalid score and invalid level separately but does not test a valid-but-inconsistent pair (e.g., score: 1, level: "critical"). A test would document the current pass-through behavior and anchor any future consistency validation.

Previous run (14)

Review

Findings

High

[missing-documentation] docs/review.md:88 — The Variables table does not include the new FULLSEND_RISK_ASSESSMENT_ENABLED environment variable. This env var is added to harness/review.yaml (env.sandbox) and controls whether the risk assessment pre-pass runs. The project’s FEATURES.md checklist (step 9) requires adding new variables to the docs.
Remediation: Add a row to the Variables table documenting FULLSEND_RISK_ASSESSMENT_ENABLED, its description, default (true), and valid values ("true", "false").

Medium

[protected-path] agents/review.md — This PR modifies 10 files under protected paths (agents/, harness/, scripts/, skills/): agents/review.md, harness/review.yaml, scripts/post-review-test.sh, scripts/post-review.sh, scripts/post-review.src.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and explains the rationale. Human approval is always required for protected-path changes.
[data-truncation] skills/pr-risk-assessment/scripts/risk-tier1.sh:32 — The PR files API call uses per_page=100 without --paginate, silently truncating the file list for PRs with >100 changed files. All derived signals (FILES_CHANGED, LINES_CHANGED, BLAST_RADIUS, PROTECTED_PATH_COUNT, etc.) are computed from an incomplete file set, biasing risk scores downward for large PRs — the PRs where accurate risk assessment matters most.
Remediation: Add --paginate to the gh api call (note: --paginate changes output to newline-delimited arrays, so downstream jq calls need jq -s 'add' to merge pages). Alternatively, document the 100-file limitation as a known constraint.
[naming-convention] harness/review.yaml:53 — The new env var FULLSEND_RISK_ASSESSMENT_ENABLED uses the FULLSEND_ prefix, but all other review-agent behavior-tuning env vars in this block use the REVIEW_ prefix (REVIEW_FINDING_SEVERITY_THRESHOLD, REVIEW_PROTECTED_PATHS). The FULLSEND_ prefix is used for infrastructure-level vars like FULLSEND_FORGE.
Remediation: Rename to REVIEW_RISK_ASSESSMENT_ENABLED and update references in skills/pr-review/SKILL.md.
[missing-documentation] docs/review.md:42 — The Control labels section does not document the new risk/* labels (risk/low, risk/moderate, risk/elevated, risk/high, risk/critical) applied by the post-script.
Remediation: Add documentation of risk/* labels to the Control labels section.

Low

[config-sync-drift] skills/pr-risk-assessment/scripts/risk-tier1.sh:17 — PROTECTED_PATHS array is hardcoded rather than reading from the REVIEW_PROTECTED_PATHS env var. Downstream users who override via harness base: composition get inconsistent behavior: post-review.sh uses the harness value, but risk-tier1.sh uses its own hardcoded list.
[shell-idiom] skills/pr-risk-assessment/scripts/risk-tier1.sh:11 — Script uses set -uo pipefail (missing -e). The intent is correct (individual signal failures fall back to UNKNOWN, documented in header), but differs from the set -euo pipefail convention in other scripts. A comment on the set line would prevent confusion.
[GHA-workflow-command-injection] scripts/post-review.sh:847 — RISK_SCORE is extracted from agent-controlled JSON via jq -r but is not validated as an integer 1–5. While it currently only flows into jq --arg (safe), adding validation would match the defensive pattern applied to RISK_LEVEL and prevent future injection if the value is interpolated into a workflow command.
[injection-vuln] skills/pr-risk-assessment/scripts/risk-tier1.sh:138 — The AUTHOR login from the GitHub API is interpolated directly into a search query URL without URL-encoding. GitHub login names are constrained to alphanumeric+hyphens, limiting practical exploitability, but URL-encoding is defense-in-depth best practice.
[scope-creep] skills/pr-review/SKILL.md:709 — The diff refactors the existing hardcoded ‘Part 3 — Doc review skill’ conditional into a generalized ‘Linked skill’ lookup table. This is a minimal prerequisite for the risk-assessment linked skill (not genuine scope creep), but the PR description should acknowledge the generalization.
[missing-documentation] docs/review.md:105 — The ‘How the agent works’ section describes the sandbox phase but does not mention the new risk-assessment pre-pass. Consistent with existing omissions of other non-standard dispatch types.
[test-coverage-gap] scripts/post-review-test.sh:1645 — Risk assessment test suite covers happy paths but does not test: invalid risk level values triggering the warning fallback, stale risk label removal when level changes, or fullsend post-comment failure graceful degradation.
[naming-convention] skills/pr-review/SKILL.md:809 — Spawn instruction says ‘model: sonnet’ but the actual sub-agent frontmatter declares model: claude-sonnet-4-6@default. The parenthetical claim ‘(from the sub-agent frontmatter)’ is factually incorrect.
[file-organization] scripts/risk-tier1-test.sh — Test file lives in scripts/ but tests a script under skills/pr-risk-assessment/scripts/. This is the only cross-directory test relationship in the codebase.
[code-organization] eval/review/cases/001-risk-low-typo-fix/annotations.yaml — New eval cases use numbers 001 and 002, while the existing case is 003, reversing chronological ordering.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (15)

Review

Findings

Medium

[truncated-api-response] skills/pr-risk-assessment/scripts/risk-tier1.sh:33 — The PR files API call uses ?per_page=100 without --paginate, so PRs with more than 100 changed files will have truncated results. FILES_CHANGED is computed from the array length (not the API’s total count), capping at 100. All downstream signals (LINES_CHANGED, PROTECTED_PATH_COUNT, SECURITY_SENSITIVE_COUNT, BLAST_RADIUS, TEST_FILE_RATIO) will undercount, systematically producing lower risk scores for the very PRs that most need elevated risk signals. The rest of the codebase uses --paginate for similar API calls.
Remediation: Use gh api --paginate to fetch all pages and merge with jq -s 'add'.
[scope-authorization] harness/review.yaml — The new skill skills/pr-risk-assessment is not listed in the harness skills: array despite being referenced by the risk-assessment sub-agent. The existing docs-review skill — which serves an analogous linked-skill role for the docs-currency sub-agent — IS listed in the skills: array (line 20). If the harness bundles skill files into the sandbox based on the skills: array, the pr-risk-assessment/SKILL.md and pr-risk-assessment/scripts/risk-tier1.sh files may not be available at runtime, causing the sub-agent to fail silently.
Remediation: Add skills/pr-risk-assessment to the harness skills: array.
[missing-env-var-docs] docs/review.md — The Variables table does not include the new FULLSEND_RISK_ASSESSMENT_ENABLED environment variable added in harness/review.yaml. Users who want to disable risk assessment (or understand its default-on behavior) have no documentation reference for this configuration knob.
Remediation: Add a row to the Variables table documenting the variable, its default (true), and valid values.
[missing-label-docs] docs/review.md — The Control labels section documents outcome labels (ready-for-merge, requires-manual-review, rejected) but does not document the new risk/* labels (risk/low, risk/moderate, risk/elevated, risk/high, risk/critical) applied by the post-review script. These labels are applied automatically when risk assessment is enabled.
Remediation: Add a paragraph or table rows documenting the risk/* labels and noting they are informational.
[protected-path] agents/, harness/, scripts/, skills/ — This PR modifies 10 files under protected paths: agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review.src.sh, scripts/post-review-test.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

[documentation-correctness] skills/pr-review/SKILL.md — Step 3c-2 says FULLSEND_RISK_ASSESSMENT_ENABLED defaults to true ("the default") but also says "If the env var is false or empty, skip this step." The harness provides the default via env.sandbox, so the wording is not contradictory in practice, but could be clearer about the distinction between "unset" and "harness-provided default."
[input-sanitization] scripts/post-review.sh — RISK_SCORE is extracted via jq -r but receives no sanitization or integer validation, unlike RISK_LEVEL which is sanitized (newline/CR/percent/colon stripping) and allowlist-validated. Same issue in scripts/post-review.src.sh. Upstream schema validation mitigates the risk, but the asymmetric sanitization is a defense-in-depth gap.
[input-sanitization] skills/pr-risk-assessment/scripts/risk-tier1.sh:143 — AUTHOR variable from GitHub API (.user.login) is interpolated directly into a gh api search URL without URL-encoding. GitHub restricts login names to [a-zA-Z0-9-], so exploitability is minimal.
[naming-convention] harness/review.yaml:53 — FULLSEND_RISK_ASSESSMENT_ENABLED uses the FULLSEND_ prefix, but all other agent-behavior-tuning defaults in this harness use the REVIEW_ prefix (e.g., REVIEW_FINDING_SEVERITY_THRESHOLD, REVIEW_PROTECTED_PATHS). The FULLSEND_ prefix is used for forge/infra plumbing (FULLSEND_FORGE).
[scope-creep] skills/pr-review/SKILL.md:709 — The skill-loading mechanism was generalized from a docs-currency-specific conditional into a generic skill-loading table. The behavioral effect for docs-currency should be identical, but the structural change extends slightly beyond the minimum scope.
[tool-list-convention] skills/pr-review/sub-agents/risk-assessment.md:7 — The tools frontmatter lists Read, Bash, Grep, omitting Glob which all seven other sub-agents include.
[incomplete-agent-description] docs/review.md:105 — The "How the agent works" section enumerates review dimensions but does not mention the new risk-assessment pre-pass sub-agent.
[schema-completeness] schemas/review-result.schema.json:219 — The schema does not enforce consistency between risk_assessment.score and risk_assessment.level (e.g., score=1 with level="critical" would pass validation).

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (16)

Review

Findings

High

[api-contract] scripts/post-review.sh:869 — The risk assessment label block uses raw gh pr edit and gh label create commands instead of the forge-abstracted forge_remove_label_edit, forge_create_label, and forge_add_label_edit functions used by the outcome-label and contextual-label sections in the same script. On GitLab, these calls fail silently (|| true / 2>/dev/null), meaning risk labels are never applied for GitLab-forged reviews. The block also references REPO_FULL_NAME instead of the script-local REPO variable set by forge_parse_pr_url().
Remediation: Replace raw gh calls with forge_remove_label_edit, forge_create_label, forge_add_label_edit. Use ${REPO} instead of ${REPO_FULL_NAME}.

Medium

[protected-path] agents/review.md — 9 files under protected paths are modified: agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review-test.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and provides rationale. Human approval is always required for protected-path changes regardless of context.
[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh:35 — PR files API call uses per_page=100 without --paginate. PRs with >100 changed files will have truncated file lists, understating all downstream risk signals (FILES_CHANGED, BLAST_RADIUS, PROTECTED_PATH_COUNT, etc.).
Remediation: Add --paginate to the gh api call.
[missing-test] scripts/post-review-test.sh:1630 — All risk assessment tests set FULLSEND_FORGE="github". No GitLab integration tests for risk label application or comment posting, despite the run_gitlab_label_test pattern existing for other features.
Remediation: Add GitLab risk tests using the run_gitlab_label_test pattern.
[pattern-inconsistency] scripts/post-review-test.sh:405 — Mock gh handlers for risk labels pattern-match raw gh CLI calls rather than forge-abstracted functions, coupling tests to the GitHub-only implementation.
Remediation: Update mock handlers and assertions once post-review.sh uses forge abstractions.
[missing-documentation] docs/review.md:88 — Variables table does not document FULLSEND_RISK_ASSESSMENT_ENABLED, added to harness/review.yaml env.sandbox with default "true".
Remediation: Add a row to the Variables table.
[missing-documentation] docs/review.md:42 — Labels documentation does not mention the new risk/* labels (risk/low through risk/critical) applied by the post-script. Note: these are informational labels, not control labels per REVIEW_CONTROL_LABELS.
Remediation: Document risk/* labels in the appropriate section.

Low

[scope-creep] skills/pr-review/SKILL.md — Linked issue recommends phased architecture; PR delivers all three tiers at once. Advisory concern — phasing was recommended, not required.
[data-exposure] scripts/post-review.sh:891 — RISK_RATIONALE sanitization strips HTML and escapes pipes but does not neutralize Markdown link/image syntax. Low risk: rationale originates from agent JSON output, and GitHub CSP mitigates exploitation.
[injection-vuln] skills/pr-risk-assessment/scripts/risk-tier1.sh:67 — AUTHOR variable interpolated unencoded into search API query string. Not exploitable due to GitHub username character restrictions, but a defense-in-depth gap.
[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh — First-time contributor detection queries is:merged PRs. If script runs post-merge, single-PR authors would be misidentified. Minor given stated pre-merge use case.
[pattern-inconsistency] skills/pr-review/SKILL.md:713 — Skill-loading table in step 4 includes risk-assessment, but risk-assessment is dispatched in step 3c-2 (pre-pass), not step 4's parallel loop.
[stale-documentation] docs/review.md:105 — "How the agent works" does not mention the new risk assessment pre-pass.

Info

[provenance-warning] Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

Previous run (17)

Review

Findings

Medium

[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh:116 — The GitHub API call to fetch PR files uses per_page=100 without --paginate. For PRs with more than 100 changed files, the file list will be silently truncated to the first 100 entries, causing all Tier 1 signals to be computed on an incomplete file set. Since this script is the sole data source for Tier 1 signals, a truncated file list would produce an underestimated risk score for large PRs.
Remediation: Add --paginate to the gh api call or document the 100-file limit as a known limitation.
[injection-vuln] scripts/post-review.sh:547 — RISK_RATIONALE from agent result JSON is sanitized with sed 's/<[^>]*>//g; s/|/\\|/g' which strips HTML tags and escapes pipes, but does not apply the standard newline/percent/colon sanitization pattern used elsewhere in the script (e.g., for RISK_LEVEL). The value flows into a GitHub comment where incomplete sanitization could produce unexpected rendering.
Remediation: Truncate RISK_RATIONALE to a reasonable length (e.g., 500 characters) and apply the standard sanitization pattern used for RISK_LEVEL.
[pattern-inconsistency] skills/pr-review/SKILL.md:590 — The linked skill table in Part 3 of step 4 lists risk-assessment alongside docs-currency. However, risk-assessment is a pre-pass sub-agent that runs in step 3c-2 and is explicitly excluded from step 4's parallel dispatch loop. Including it in step 4's linked-skill table creates a contradictory instruction — the table tells the orchestrator to handle risk-assessment in step 4 while the surrounding prose says not to dispatch it there. Step 3c-2 already has its own instruction to read the linked skill.
Remediation: Move the risk-assessment row out of step 4's Part 3 table. The step 4 table should only contain linked skills for dimension sub-agents dispatched in that step.
[incomplete-doc] docs/review.md:88 — The Variables table does not document the new FULLSEND_RISK_ASSESSMENT_ENABLED env var introduced in harness/review.yaml. Operators who want to disable risk scoring need to know this knob exists and how to override it via harness composition.
Remediation: Add a row to the Variables table for FULLSEND_RISK_ASSESSMENT_ENABLED.
[incomplete-doc] docs/review.md:42 — The Control labels section does not mention the new risk/* labels (risk/low through risk/critical) applied by the post-review script. Users will see these labels on their PRs but have no documentation explaining what they mean or that they are informational.
Remediation: Add a paragraph describing risk labels, their meaning, and that they are informational.
[protected-path] agents/review.md, harness/review.yaml, scripts/post-review.sh, scripts/post-review-test.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh — 9 files under protected paths (agents/, harness/, scripts/, skills/). The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and explains the rationale for the changes. Human approval is always required for protected-path changes, regardless of context.

Low

[naming-convention] skills/pr-review/sub-agents/risk-assessment.md:6 — The sub-agent frontmatter specifies model: claude-sonnet-4-6@default, but SKILL.md step 3c-2 item 4 instructs the orchestrator to spawn with model: sonnet. The established dispatch pattern in step 4 reads the model from the sub-agent frontmatter. The parenthetical "(from the sub-agent frontmatter)" in step 3c-2 clarifies intent but the hardcoded alias breaks the indirection pattern.
Remediation: Change SKILL.md step 3c-2 to reference the frontmatter value rather than hardcoding sonnet.
[scope-creep] skills/pr-review/SKILL.md — The issue (Add PR-level risk assessment score to the review pipeline fullsend#4698) describes the risk score informing review effort and gating auto-merge. The PR explicitly narrows scope to "purely informational — does not gate the review outcome." While incremental delivery is acceptable, the deferred scope is not tracked with a follow-up issue.
Remediation: Note that review-effort tuning and auto-merge gating are planned follow-ups, ideally with a tracking issue.
[incomplete-doc] docs/review.md:105 — The "How the agent works" section describes sub-agent dispatch but does not mention the risk-assessment pre-pass.
Remediation: Add a brief mention of the risk assessment pre-pass.
[pattern-inconsistency] scripts/post-review.sh:507 — The risk-level sanitization comment says "(same pattern as lines 113-116)". Line-number references in comments are brittle. Other sections in this file use descriptive references.
Remediation: Replace with a descriptive reference.
[injection-vuln] scripts/post-review.sh:536 — RISK_SCORE receives no sanitization unlike RISK_LEVEL. While jq --arg prevents injection and the schema constrains the value upstream, the inconsistency is a defense-in-depth gap.
Remediation: Validate that RISK_SCORE matches ^[1-5]$ before use.
[injection-vuln] skills/pr-risk-assessment/scripts/risk-tier1.sh:93 — AUTHOR variable extracted from GitHub API is interpolated into a search query URL without validation. GitHub usernames have strict constraints and gh api URL-encodes, but the pattern is inconsistent with the script's otherwise careful handling.
Remediation: Validate AUTHOR against a strict regex before interpolating.
[edge-case] skills/pr-risk-assessment/SKILL.md:148 — The Tier 2 "Code age/stability" and "Churn hotspot" dimensions pull in opposite directions for the same file. Both are internally consistent but the interaction could confuse implementers.
[design-direction] harness/review.yaml — FULLSEND_RISK_ASSESSMENT_ENABLED is in env.sandbox only, not env.runner. The post-script reads risk_assessment from the result JSON rather than checking the flag. This asymmetry is architecturally sound but could surprise future maintainers.

Info

[provenance-warning] — Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run (18)

Review

Findings

Medium

[logic-error] skills/pr-review/SKILL.md:732 — Step 4's dispatch exclusion list names only security-triage and challenger but omits risk-assessment, which runs in step 3c-2 as a pre-pass. The skill-loading table at line 764 includes risk-assessment with a linked skill, so an orchestrator following step 4 literally could attempt to dispatch it in the parallel loop alongside dimension sub-agents, resulting in double dispatch.
Remediation: Update the exclusion list to include risk-assessment which runs in step 3c-2.
[workflow-command-injection] scripts/post-review.sh:516 — RISK_LEVEL is sanitized for newlines (lines 510–511) but not for % or : characters before interpolation into the ::warning:: GHA workflow command. The established sanitization pattern in this file (lines 113–116 for REVIEW_FINDING_SEVERITY_THRESHOLD, lines 227–230 for REVIEW_PROTECTED_PATHS) strips these characters to prevent workflow command injection via %0A, ::set-env::, etc.
Remediation: Add RISK_LEVEL="${RISK_LEVEL//%/}" and RISK_LEVEL="${RISK_LEVEL//:/}" after line 511.
[stale-doc] docs/review.md:92 — The Variables section does not document the new FULLSEND_RISK_ASSESSMENT_ENABLED environment variable added in harness/review.yaml.
Remediation: Add a row to the Variables table documenting the new env var, its default (true), and valid values.
[stale-doc] docs/review.md:42 — The control labels section does not document the new risk/* labels (risk/low, risk/moderate, risk/elevated, risk/high, risk/critical) introduced by post-review.sh.
Remediation: Add documentation for the risk label family.
[protected-path] harness/, scripts/, skills/ — This PR modifies 8 files under protected paths: harness/review.yaml, scripts/post-review.sh, scripts/post-review-test.sh, scripts/risk-tier1-test.sh, skills/pr-review/SKILL.md, skills/pr-review/sub-agents/risk-assessment.md, skills/pr-risk-assessment/SKILL.md, skills/pr-risk-assessment/scripts/risk-tier1.sh. The PR links to Add PR-level risk assessment score to the review pipeline fullsend#4698 and provides rationale for modifying governance and infrastructure files. Human approval is always required for protected-path changes, regardless of context.

Low

[logic-error] skills/pr-review/SKILL.md:328 — Step 3c's preamble lists only challenger as an exception to parallel dispatch but does not mention risk-assessment. The non-standard dispatch description (lines 50–57) correctly identifies it as a pre-pass. Documentation consistency issue.
Remediation: Add risk-assessment to the preamble's exception list.
[sync-comment-accuracy] skills/pr-risk-assessment/scripts/risk-tier1.sh:16 — Sync comment references post-review.sh line 167–186 but the protected paths list is defined in harness/review.yaml (env.sandbox.REVIEW_PROTECTED_PATHS). The comment directs maintainers to the wrong source.
Remediation: Reference harness/review.yaml for the canonical definition.
[sync-comment-accuracy] scripts/risk-tier1-test.sh:461 — Same sync comment issue as risk-tier1.sh — references post-review.sh instead of harness/review.yaml.
[missing-test] scripts/post-review-test.sh — No test exercises the invalid risk level fallback path in post-review.sh (lines 513–518). The case-statement allowlist rejects invalid levels and emits a ::warning::, but this code path has no test coverage.
Remediation: Add a test case with risk_assessment containing an invalid level (e.g., "level": "bogus").
[edge-case] skills/pr-risk-assessment/scripts/risk-tier1.sh:32 — PR files API call fetches only the first page (per_page=100) without --paginate. For PRs with >100 changed files, all signal counts will be computed from truncated data. Consistent with existing codebase patterns but risk signals will be systematically understated for very large PRs.
[stale-doc] docs/review.md:103 — The "How the agent works" section does not mention the risk-assessment pre-pass capability.
[intent-coherence] eval/review/cases/001-risk-low-typo-fix/annotations.yaml:10 — The eval case's forbidden label list omits risk/moderate and risk/elevated. A typo fix should produce score 1 (low), so adjacent levels should also be forbidden to catch scoring errors.
Remediation: Add risk/moderate and risk/elevated to the forbidden list.

Labels: PR adds risk assessment feature to the review pipeline, modifying review agent components

Next steps:

/fs-fix — agent addresses review findings automatically
/fs-fix <your instruction> — agent fixes with your specific guidance
Push commits directly — review re-runs automatically on push
/fs-fix-stop — disable automatic fix runs for this PR

waynesun09

Review sweep — risk assessment pipeline

Review-only pass. 8 findings are posted inline below; 2 more could not be anchored to a line in this diff and are recorded here.

MEDIUM — `scripts/risk-tier1-test.sh` is not wired into CI (`Makefile:43`, not in this diff)

Verified at head: the script-test target (Makefile lines 43–65) enumerates every test script explicitly — bundle-sh-test.sh, post-review-test.sh, pre-review-test.sh, validate-output-schema-test.sh, etc. — with no globbing. scripts/risk-tier1-test.sh is not in the list, so .github/workflows/script-test.yml never executes it. The test plan claims "bash scripts/risk-tier1-test.sh — 30 unit tests pass", but that only happens when a human runs it locally; regressions in the new risk logic will not be caught in CI.

Suggestion: Add $(call run-timed,bash scripts/risk-tier1-test.sh) to the script-test target, and consider chmod +x on the new script and test to match the 100755 mode used by the other scripts in scripts/.

MEDIUM — premature-decision: PR closes #4698 while shipping a strict subset, and the description misstates where the feature flag lives (PR-level)

Verified against the issue and the diff.

(a) The PR body's changes table says the feature flag lives in env/review.env "with shell defaulting" — env/review.env does not exist in this repo at all (ls env/ → gcp-vertex.env, github, gitlab, ssl-cainfo.env), the file is not in the 16-file diff, and grep -rn FULLSEND_RISK_ASSESSMENT_ENABLED finds it only as a hardcoded literal at harness/review.yaml:57 plus prose in skills/pr-review/SKILL.md. There is no ${VAR:-true} anywhere, so the "default true" documented at SKILL.md:563 is a harness literal, not a default.

(b) Closes fullsend-ai/fullsend#4698 auto-closes cross-repo on merge, but the issue asks the score to "Inform review effort: model selection (sonnet vs opus), sub-agent count, and whether human approval is required" and to "Gate auto-merge eligibility: only low-risk PRs qualify", while this PR states "Score is purely informational — does not gate the review outcome". The issue's Architecture recommendation is phase 1 "Tier 1 signals computed in pre-review.sh with no LLM cost", whereas this runs the script inside an LLM sub-agent that also performs the weighted arithmetic. The issue's taxonomy is 1–2 low / 3 medium / 4 high / 5 critical vs the shipped low/moderate/elevated/high/critical, and the issue's path-sensitivity dimension names .pem and .key, which are absent from SECURITY_PATTERNS.

(c) Both end-to-end test-plan boxes ("Run review eval cases", "Manual test: trigger review on a test PR") are unchecked — a single live run would have surfaced the unregistered skill and the depth-1 git history immediately.

Suggestion: Change Closes fullsend-ai/fullsend#4698 to Part of fullsend-ai/fullsend#4698 and add a short "Deviations from the issue" section covering the informational-only scope, the sub-agent-vs-pre-review placement, the level-taxonomy rename, and the dropped .pem/.key patterns, so the remaining phases stay tracked. Correct the changes table to point at harness/review.yaml and drop the "shell defaulting" claim (or implement a real default). Run the two eval cases and one live PR before merge and check the boxes, or mark the PR draft.

Note: the harness/review.yaml comment is anchored on the only changed line in that file; the defect is in the skills: list near line 17.

maruiz93 · 2026-08-19T15:46:14Z

Addressed in b461fe4:

Fixed jq Cartesian product in LINES_CHANGED (was cross-multiplying across files)
Added % and : sanitization to RISK_LEVEL (matching established pattern at lines 113-116)
Added risk-assessment to dispatch exclusion lists in steps 3c and 4
Updated sync comments to reference harness/review.yaml
Added risk_assessment to review.md output field table
Tightened eval case forbidden labels

The pagination (>100 files) and path matching refinements are consistent with existing codebase patterns and will be addressed in follow-up work.

fullsend-ai-review · 2026-08-19T15:48:12Z

🤖 Finished Review · ✅ Success · Started 3:48 PM UTC · Completed 4:07 PM UTC

Commit: b461fe4 · View workflow run →

Superseded by updated review

fullsend-ai-review · 2026-08-19T16:24:12Z

🤖 Finished Review · ✅ Success · Started 4:24 PM UTC · Completed 4:42 PM UTC

Commit: bd98ef7 · View workflow run →

waynesun09

Review-only sweep (additional pass). 3 findings below; 3 others from this batch were already covered by existing review comments/threads and were skipped as duplicates.

fullsend-ai-review · 2026-08-20T09:39:13Z

🤖 Review · ⚠️ Cancelled · Started 9:39 AM UTC · Ended 9:43 AM UTC

Commit: 1d69b2c · View workflow run →

fullsend-ai-review · 2026-08-20T09:45:10Z

🤖 Finished Review · ✅ Success · Started 9:45 AM UTC · Completed 10:06 AM UTC

Commit: ec0580f · View workflow run →

fullsend-ai-review · 2026-08-20T14:06:38Z

🤖 Finished Review · ✅ Success · Started 2:06 PM UTC · Completed 2:28 PM UTC

Commit: 3b315df · View workflow run →

waynesun09

Additional review-only findings (scoring formula and risk-assessment skill design), verified and deduplicated against existing comments on this PR.

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>

- Add FIRST_TIME_CONTRIBUTOR e2e test coverage (pagination test) - Add *.spec.* to test-file glob patterns with unit test - Specify explicit weight-redistribution percentages for all degenerate tier combinations in SKILL.md - Align protected-path matching to prefix semantics (matches post-review) - Make REVIEW_GIT_FETCH_DEPTH conditional on risk assessment enablement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>

fullsend-ai-review · 2026-08-25T08:02:56Z

🤖 Finished Review · ✅ Success · Started 8:02 AM UTC · Completed 8:22 AM UTC

Commit: 77b8d15 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.51

…ocs wording The pre-script runs on the runner and checks REVIEW_RISK_ASSESSMENT_ENABLED to auto-default REVIEW_GIT_FETCH_DEPTH. Without the variable in env.runner, clone deepening never triggers in pipeline runs. Also fix "either variable" wording in docs to "any variable" since the table now has six entries. Signed-off-by: Marta Anon <maruiz93@users.noreply.github.com> Signed-off-by: Marta Anon <manon@redhat.com>

fullsend-ai-review · 2026-08-25T08:28:43Z

🤖 Finished Review · ✅ Success · Started 8:28 AM UTC · Completed 8:46 AM UTC

Commit: 1daa21d · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.59

Superseded by updated review

waynesun09

Re-verified at head 1daa21d (local checkout, all prior rounds cross-checked against the code, not the reply text). Everything I raised earlier is confirmed fixed or explicitly deferred (#922):

scripts/risk-tier1-test.sh sources the shipped script; e2e stubs gh and runs main ✔ · post-review-test.sh 17 distinct risk cases ✔ · pre-review-test.sh ✔ · bundles regenerate byte-identical from .src.sh ✔
functional-tests (review) on this head: the Tier 1 script demonstrably ran inside the sandbox for all three cases (full 10-signal block in the transcripts — FILES_CHANGED=2/3/1, SECURITY_SENSITIVE_COUNT=2 for 002) and real labels landed (risk/low on 001, risk/moderate on 002). All six judges pass, including risk_label_present.
Empirically checked git fetch --unshallow <url> on a depth-1 SHA checkout: HEAD's history goes 1 → 1165 commits and per-file git log works, so Tier 2 is viable. FETCH_HEAD is anonymized by git (no token recorded).
The bot's "duplicate truncated test names" finding is a display-masking artifact (#462) — names are unique.

One thing holds my approval (inline on harness/review.yaml): REVIEW_RISK_ASSESSMENT_ENABLED: "true" lives in the top-level env: block, so it is on for GitLab too — contrary to the resolved-thread reply that it is "only set in the GitHub forge section". On a GitLab MR the sub-agent still runs: Tier 1 is all UNKNOWN (no gh/GH_TOKEN), the clone is never deepened (FULLSEND_FORGE != github → Tier 2 unavailable), and SKILL.md's fallback ("all absent → Tier 1 = 3") yields a fabricated risk/elevated label on every MR, while the sticky comment silently fails (fullsend post-comment is GitHub-only). Moving the two keys under forge.github.env makes the deferral to #922 true. I'll approve as soon as that lands.

Known limitation to keep on record (not blocking): both new eval cases still exit 1 at this head — the fixture PR is authored by the reviewing account, and both agents chose request-changes, so the 422 fires regardless of the CODEOWNERS/protected-path downgrade (which only converts approve → comment). Run-level exit is noise; the judges are the real gate, and risk_label_present closes the "no-op passes" hole.

Nits (take or leave):

scripts/validate-output-schema-test.sh has no risk_assessment cases — the five allOf score↔level if/then blocks are untested.
agents/review.md frontmatter skills: doesn't list pr-risk-assessment (the harness skills: array is what actually uploads, so this is documentation drift).
pre-review.src.sh could keep the token out of argv with git -c http.extraheader="AUTHORIZATION: basic $(printf 'x-access-token:%s' "$GH_TOKEN" | base64 -w0)" fetch --unshallow origin — FETCH_HEAD is already clean, so this is optional.

Second-opinion pass (Grok) was run on this head; its other findings were checked and refuted against the code: post-scripts only run after schema validation passes (ADR 0022), so a non-string rationale cannot reach the jq slice; and risk_assessment did reach agent-result.json in every eval case.

Move REVIEW_RISK_ASSESSMENT_ENABLED from the top-level env block (which applies to all forges) to forge.github.env.runner and forge.github.env.sandbox. On GitLab, risk-tier1.sh depends on the GitHub API and would produce fabricated scores. Signed-off-by: Marta Anon <manon@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>

fullsend-ai-review · 2026-08-25T16:26:55Z

🤖 Review · ⚠️ Cancelled · Started 4:26 PM UTC · Ended 4:42 PM UTC

Commit: e8ae971 · View workflow run →

- Add 10 risk_assessment test cases to validate-output-schema-test.sh covering all five score↔level allOf constraints plus mismatches, missing rationale, out-of-range score, and additional properties. - Add pr-risk-assessment to agents/review.md frontmatter skills list to match harness/review.yaml. - Pass GH_TOKEN via http.extraheader instead of URL in pre-review.src.sh to keep the token out of /proc/*/cmdline. Signed-off-by: Marta Anon <manon@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>

fullsend-ai-review · 2026-08-25T16:44:54Z

🤖 Finished Review · ✅ Success · Started 4:44 PM UTC · Completed 5:12 PM UTC

Commit: e686204 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.91

waynesun09

Approving at head e686204.

Re-verified the two follow-up commits:

e8ae971 — REVIEW_RISK_ASSESSMENT_ENABLED now lives only under forge.github.env.runner / forge.github.env.sandbox; the top-level env: no longer carries it, so GitLab MRs don't run the pre-pass until #922 adds a path. Resolves my one blocking thread.
e686204 — schema tests now cover the five allOf score↔level pairs plus mismatches, missing rationale, and out-of-range score (script-test green in CI, which has jsonschema); agents/review.md frontmatter lists pr-risk-assessment; the unshallow fetch uses http.extraheader (actions/checkout's pattern) so the token is out of argv. I ran that exact command shape locally on a depth-1 SHA checkout: HEAD history 1 → 1167 commits, nothing persisted in FETCH_HEAD/config. Runner default is ubuntu-24.04, so base64 -w0 is safe.

Local: risk-tier1-test, pre-review-test, post-review-test all pass; both bundles regenerate byte-identical. Prior-head eval run already proved the sub-agent → Tier 1 script → label path end-to-end; these commits don't touch that path (the eval harness never reaches the deepening branch, hence the local check above).

Known/accepted: eval cases 001/002 exit 1 on the self-review 422 (judges gate); GitLab risk path deferred to #922.

fullsend-ai-retro · 2026-08-25T17:39:29Z

🤖 Finished Retro · ✅ Success · Started 5:39 PM UTC · Completed 5:53 PM UTC

Commit: e686204 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.61

fullsend-ai-retro · 2026-08-25T17:52:59Z

Retro: PR #861 — feat(risk): add PR risk assessment scoring to review pipeline

Timeline: Aug 18–25 (8 days), 17 commits, merged by author maruiz93.

Review participants: Human reviewer waynesun09 (7 rounds, ~40 unique findings), fullsend-ai-review (~35 status comments + ~30 inline findings), qodo-code-review (13 inline findings on initial commit).

Review quality delta

The human reviewer caught 4 CRITICAL and 4 HIGH-severity issues that both automated reviewers missed entirely:

Silent no-op: The pr-risk-assessment skill was listed in agents/review.md but absent from harness/review.yaml skills array, so it was never uploaded to the sandbox.
Edits to generated file: Author modified pre-review.sh (generated output) instead of pre-review.src.sh (source). The file header says # GENERATED — DO NOT EDIT but neither agent flagged this.
Eval cases that cannot pass: Eval case 002's expected score range was mathematically impossible under the shipped scoring formula. Both eval cases also had structural issues making them no-op validations.
Tests exercising copy-pasted logic: 30 unit tests sourced a duplicated copy of functions rather than the shipped script.
Shallow clone breaks git-history signals: Tier 2 signals rely on git log but CI runs with fetch-depth: 1.
Overly broad CI file detection: has_ci_files treated every .github/* path as CI-relevant.
Test ratio misses naming conventions: compute_test_ratio missed .spec. file convention.

All missed findings share a common trait: they require reasoning about how code is built, deployed, and executed — not just what the code says. The review agent excels at single-file code correctness, pattern-matching for known vulnerability categories, and severity calibration. It struggles with build-artifact provenance, CI environment modeling, and mathematical verification of test expectations.

What worked well

Qodo caught 13 real issues on the initial commit (Cartesian product bug in jq, pagination gap, sanitization gaps). All were addressed.
fullsend-ai-review found a unique logic error about REVIEW_RISK_ASSESSMENT_ENABLED env var placement (commit 77b8d15).
The challenger sub-agent correctly removed a false-positive test-adequacy finding.
All ~70 unique findings across all reviewers were addressed or explicitly acknowledged.

Cancelled run waste

17 of 36 review runs were cancelled (47%), wasting an estimated 88+ minutes of agent compute. The worst episode was 11 consecutive cancellations in 75 minutes on Aug 21 during a rapid fix-push cycle. This is additional evidence for existing debounce issues: fullsend-ai/fullsend#4960, fullsend-ai/fullsend#1014, fullsend-ai/fullsend#4069. Issue fullsend-ai/fullsend#6573 (debounce during rapid human pushes) was closed on Aug 24, suggesting active progress.

Repeated false positive

fullsend-ai-review flagged "truncated test names with ellipsis" 4 separate times across different review runs. The author explained this is a context-packaging artifact (tracked in agents#462), not actual truncation.

Existing issue evidence

agents#317 (correctness sub-agent deployment-context feasibility): This retro provides additional evidence — the skill-not-in-harness and shallow-clone findings are deployment-context issues the sub-agent missed.
agents#302 (self-report coverage gaps on large PRs): With 24 changed files and 1,543 additions, this PR exceeded the agent's coverage capacity. The agent did not flag this gap.
agents#462 (context abbreviation causing false positives): The repeated truncated-test-name false positive is a direct instance of this issue.

Proposals filed

Correctness sub-agent should detect edits to generated files and verify test targets (in fullsend-ai/agents)
Correctness sub-agent should verify eval and test fixture expectations are satisfiable (in fullsend-ai/agents)

maruiz93 requested a review from a team as a code owner August 18, 2026 08:10

maruiz93 force-pushed the 4698-pr-risk-assessment branch from c848b26 to d2f4860 Compare August 18, 2026 08:17

qodo-code-review Bot reviewed Aug 18, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added review-agent feature labels Aug 18, 2026

waynesun09 reviewed Aug 18, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 19, 2026

maruiz93 force-pushed the 4698-pr-risk-assessment branch from b461fe4 to bd98ef7 Compare August 19, 2026 16:21

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 19, 2026

waynesun09 reviewed Aug 19, 2026

View reviewed changes

Comment thread scripts/post-review.sh Outdated

Comment thread scripts/post-review.sh Outdated

Comment thread skills/pr-risk-assessment/scripts/risk-tier1.sh Outdated

maruiz93 force-pushed the 4698-pr-risk-assessment branch from bd98ef7 to 1d69b2c Compare August 20, 2026 09:37

maruiz93 force-pushed the 4698-pr-risk-assessment branch from 1d69b2c to ec0580f Compare August 20, 2026 09:42

This comment was marked as outdated.

Sign in to view

maruiz93 force-pushed the 4698-pr-risk-assessment branch from ec0580f to 3b315df Compare August 20, 2026 14:04

waynesun09 reviewed Aug 20, 2026

View reviewed changes

Comment thread eval/review/cases/002-risk-high-auth-change/annotations.yaml

Comment thread skills/pr-risk-assessment/SKILL.md Outdated

Comment thread skills/pr-risk-assessment/SKILL.md

Comment thread skills/pr-risk-assessment/SKILL.md

maruiz93 and others added 2 commits August 25, 2026 10:01

maruiz93 force-pushed the 4698-pr-risk-assessment branch from e17e22d to 77b8d15 Compare August 25, 2026 08:01

maruiz93 requested a review from waynesun09 August 25, 2026 08:01

This comment was marked as outdated.

Sign in to view

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 25, 2026

waynesun09 reviewed Aug 25, 2026

View reviewed changes

Comment thread harness/review.yaml Outdated

waynesun09 approved these changes Aug 25, 2026

View reviewed changes

maruiz93 added this pull request to the merge queue Aug 25, 2026

Merged via the queue into fullsend-ai:main with commit f20826e Aug 25, 2026
28 checks passed

maruiz93 deleted the 4698-pr-risk-assessment branch August 25, 2026 17:37

fullsend-ai-retro Bot mentioned this pull request Aug 25, 2026

Correctness sub-agent should detect edits to generated files and verify test targets #1022

Open

fullsend-ai-retro Bot mentioned this pull request Aug 25, 2026

Correctness sub-agent should verify eval and test fixture expectations are satisfiable #1023

Open

waynesun09 mentioned this pull request Aug 25, 2026

review: pre-review unshallow (#861) adds 1–18 min per run on large repos; breaks fullsend e2e settle window #1032

Open

fullsend-ai-coder Bot mentioned this pull request Aug 25, 2026

perf(#1032): use blobless fetch with timeout for pre-review clone deepening #1033

Open

maruiz93 mentioned this pull request Aug 26, 2026

Risk assessment sub-agent produces inconsistent scores across re-reviews with identical rationale #1037

Closed

qodo-code-review Bot mentioned this pull request Sep 1, 2026

feat(review): filter unreviewable content from diff context #1008

Open

guyoron1 mentioned this pull request Sep 6, 2026

feat(review): route trivial diffs to a cheaper model fullsend-ai/fullsend#6590

Closed

Uh oh!

Conversation

maruiz93 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Deviations from issue #4698

Test plan

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 18, 2026

PR Summary by Qodo

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

Medium

Low

Review

Findings

maruiz93 commented Aug 18, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 18, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 18, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 18, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 18, 2026 •

edited

Loading

MEDIUM — `scripts/risk-tier1-test.sh` is not wired into CI (`Makefile:43`, not in this diff)

fullsend-ai-review Bot commented Aug 19, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 19, 2026 •

edited

Loading