Skip to content

refactor(harness): migrate review agent to env.runner/env.sandbox (ADR 0055) #2758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ralphbean merged 1 commit into from
Jul 2, 2026
+19 −18
Merged

refactor(harness): migrate review agent to env.runner/env.sandbox (ADR 0055) #2758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions internal/scaffold/fullsend-repo/env/review.env
View file
Open in desktop

This file was deleted.

28 changes: 18 additions & 10 deletions internal/scaffold/fullsend-repo/harness/review.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ host_files:
- src: ${GCP_OIDC_TOKEN_FILE}
dest: /sandbox/workspace/.gcp-oidc-token
optional: true
- src: env/review.env
dest: /sandbox/workspace/.env.d/review.env
expand: true
- src: ${PRIOR_REVIEW_FILE}
dest: /sandbox/workspace/prior-review.txt
optional: true
Expand All @@ -37,17 +34,28 @@ validation_loop:
script: scripts/validate-output-schema.sh
max_iterations: 2

runner_env:
FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/review-result.schema.json
env:
runner:
FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/review-result.schema.json
sandbox:
PRIOR_REVIEW_SHA: "${PRIOR_REVIEW_SHA}"
PRIOR_REVIEW_PROVENANCE: "${PRIOR_REVIEW_PROVENANCE}"
REVIEW_FINDING_SEVERITY_THRESHOLD: "${REVIEW_FINDING_SEVERITY_THRESHOLD}"
Comment on lines +37 to +43

@qodo-code-review qodo-code-review Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Unset threshold fails validation 🐞 Bug ☼ Reliability

internal/scaffold/fullsend-repo/harness/review.yaml now references
${REVIEW_FINDING_SEVERITY_THRESHOLD} in env.sandbox, and fullsend run validates that every
${VAR} reference exists in the host environment. The reusable review workflow does not define
REVIEW_FINDING_SEVERITY_THRESHOLD, so the review job can fail early with an environment validation
error.
Agent Prompt 
## Issue description
`fullsend run` calls `Harness.ValidateRunnerEnvWith(os.LookupEnv)` which errors when any `${VAR}` reference in `env.runner`/`env.sandbox` is *unset* (missing from the environment, even if conceptually optional).

This PR introduces `env.sandbox.REVIEW_FINDING_SEVERITY_THRESHOLD: "${REVIEW_FINDING_SEVERITY_THRESHOLD}"` but the reusable workflow that runs the review agent does not define `REVIEW_FINDING_SEVERITY_THRESHOLD` at all, so the workflow can fail before running pre_script/agent.

## Issue Context
`REVIEW_FINDING_SEVERITY_THRESHOLD` is documented as optional (empty/unset should behave like `low`), but the runner’s validation requires the host variable to exist if referenced.

## Fix Focus Areas
- .github/workflows/reusable-review.yml[167-177]
- internal/scaffold/fullsend-repo/harness/review.yaml[37-43]

## Suggested fix
Add `REVIEW_FINDING_SEVERITY_THRESHOLD` to the reusable workflow env for the “Run review agent” step, setting it to an empty string by default (or to a repo/org variable), e.g.:

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

This ensures the variable is always present (possibly empty), satisfying validation while preserving optional behavior.

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


timeout_minutes: 20

forge:
github:
pre_script: scripts/pre-review.sh
post_script: scripts/post-review.sh
runner_env:
REVIEW_TOKEN: "${REVIEW_TOKEN}"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
PR_NUMBER: "${PR_NUMBER}"
GITHUB_PR_URL: "${GITHUB_PR_URL}"
env:
runner:
REVIEW_TOKEN: "${REVIEW_TOKEN}"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
PR_NUMBER: "${PR_NUMBER}"
GITHUB_PR_URL: "${GITHUB_PR_URL}"
sandbox:
GITHUB_PR_URL: "${GITHUB_PR_URL}"
GH_TOKEN: "${GH_TOKEN}"
PR_NUMBER: "${PR_NUMBER}"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
2 changes: 1 addition & 1 deletion internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ If `PRIOR_REVIEW_SHA` is non-empty, compute the set of files that
changed since the prior review:

```bash
# REPO_FULL_NAME and PR_NUMBER are set in env/review.env
# REPO_FULL_NAME and PR_NUMBER are set via env.sandbox in harness/review.yaml
head_SHA=$(gh api "repos/${REPO_FULL_NAME}/pulls/${PR_NUMBER}" --jq '.head.sha')
COMPARE=$(gh api "repos/${REPO_FULL_NAME}/compare/${PRIOR_REVIEW_SHA}...${head_SHA}")
TOTAL_COMMITS=$(echo "$COMPARE" | jq '.total_commits')
Expand Down
Loading