Skip to content

Fix: reusable-review.yml missing REVIEW_FINDING_SEVERITY_THRESHOLD env var #2935

Description

@fullsend-ai-retro

What happened

PR #2758 migrated the review agent harness from host_files with expand: true (which silently substitutes empty string for unset vars via os.Expand) to env.sandbox (which errors on unset vars via os.LookupEnv). The variable REVIEW_FINDING_SEVERITY_THRESHOLD was moved into env.sandbox at internal/scaffold/fullsend-repo/harness/review.yaml:43, but the reusable workflow .github/workflows/reusable-review.yml does not define it in the agent step's env: block (lines 169–176). Qodo's code review bot flagged this on Jun 29, but the review agent's final run said 'Looks good to me' and the PR was merged on Jul 2 without addressing it. Any repo using the default scaffold workflow without externally defining REVIEW_FINDING_SEVERITY_THRESHOLD (e.g., via vars.REVIEW_FINDING_SEVERITY_THRESHOLD) will hit a harness validation error before the review agent starts.

What could go better

This is a concrete bug with high confidence — the validation logic in internal/harness/harness.go:542-583 will reject any ${VAR} reference in env.sandbox where the host variable is unset. The reusable workflow is the canonical caller and does not set this variable. Repos that define it via GitHub Actions variables would be unaffected, which may explain why it hasn't caused widespread failures yet. But any new repo using the default scaffold without custom variable configuration would break on the first review dispatch.

Proposed change

Add REVIEW_FINDING_SEVERITY_THRESHOLD to the env: block of the 'Run review agent' step in .github/workflows/reusable-review.yml, defaulting to the GitHub Actions variable:

env:
  REVIEW_FINDING_SEVERITY_THRESHOLD: ${{ vars.REVIEW_FINDING_SEVERITY_THRESHOLD }}

This ensures the variable is always present (possibly empty), satisfying os.LookupEnv validation while preserving the optional behavior where empty/unset means 'low'. As ADR 0055 migration continues for other agents (triage, code, fix, retro, prioritize), audit each migration for similar unset-variable gaps.

Validation criteria

After the fix: (1) fullsend run with the review harness succeeds on a clean environment where REVIEW_FINDING_SEVERITY_THRESHOLD is not set externally. (2) The scaffold integration tests pass. (3) A new repo using the default scaffold can dispatch the review agent without env validation errors.


Generated by retro agent from #2758

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/reviewReview agentcomponent/dispatchWorkflow dispatch and triggerscomponent/harnessAgent harness, config, and skills loadinggood first issueGood for newcomerspriority/highSignificant impact, address soonready-for-triageRetro-filed issue awaiting triage agentready-to-codeTriaged and ready for the code agenttype/bugConfirmed defect in existing behavior

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions