Repository navigation
ci(eval): add functional tests for triage agent - #31
Conversation
Port the triage agent functional test suite from fullsend-ai/fullsend. Includes 4 test cases (bug, needs-info, feature-request, duplicate), the agent-eval-harness submodule, hook scripts for ephemeral repo lifecycle, and a GitHub Actions workflow with PR authorization checks. Key adaptations for this repo: - fullsend binary installed from release (no Go build step) - FULLSEND_DIR defaults to repo root (this repo IS the scaffold) - OpenShell installed from release (no pinned version file) - Authorization action is self-contained Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 5:14 PM UTC · Completed 5:30 PM UTC |
PR Summary by QodoCI: add triage agent functional test suite with ok-to-test PR gate
AI Description
Diagram
High-Level Assessment
Files changed (27)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. ::warning:: uses unsanitized FILE_COUNT
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| FILE_COUNT=$(echo "$FILES" | wc -l) | ||
| if [ "$FILE_COUNT" -ge 300 ]; then | ||
| echo "::warning::Compare API returned $FILE_COUNT files (possible truncation at 300) — running functional tests as a precaution" | ||
| echo "relevant=true" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
1. ::warning:: uses unsanitized file_count 📜 Skill insight ⛨ Security
The workflow emits a GitHub Actions workflow command with an interpolated variable ($FILE_COUNT) that is not sanitized for :: sequences, encoded newlines (%0A/%0D), ANSI escapes, or control characters. This can enable workflow-command injection if the value is ever attacker-influenced or becomes non-numeric in future edits.
Agent Prompt
## Issue description
The workflow prints a `::warning::` command that interpolates `FILE_COUNT` without sanitizing it, which violates the requirement that every interpolated value in GHA workflow commands be individually sanitized.
## Issue Context
In `.github/workflows/functional-tests.yml`, `FILE_COUNT` is inserted directly into `echo "::warning::..."`.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[98-101]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
2. pull-requests: write unjustified 📜 Skill insight ⛨ Security
The new workflow grants pull-requests: write without an explicit linked issue/ADR justifying the permission scope. This violates the least-privilege requirement for permission manifest changes.
Agent Prompt
## Issue description
The workflow introduces `pull-requests: write` which is a permission expansion and must be least-privilege with explicit justification.
## Issue Context
The gate job needs to post comments and remove labels; these operations may be possible with narrower scopes (e.g., `issues: write` plus `pull-requests: read`) depending on the exact API usage.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[45-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| name: Functional Tests | ||
|
|
||
| # PR-triggered functional tests use pull_request_target so fork PRs receive | ||
| # secrets. Authorization runs in a separate gate job (base checkout only) | ||
| # before the test job checks out the PR head — same pattern as fullsend e2e. | ||
|
|
There was a problem hiding this comment.
3. Protected .github files modified 📜 Skill insight § Compliance
This PR modifies protected governance/infrastructure paths under .github/, which must not be auto-approved and requires explicit human review. The change introduces new CI workflow and authorization automation under a protected directory.
Agent Prompt
## Issue description
Protected governance/infrastructure files under `.github/` were modified; this must be surfaced as requiring explicit human approval and should be protected by repo mechanisms.
## Issue Context
The PR adds/changes workflow and automation under `.github/`.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[1-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| thresholds: | ||
| triage_quality: | ||
| # TODO: raise to 3.5 once the triage agent critically examines source | ||
| # code instead of accepting issue descriptions at face value. | ||
| min_mean: 2.5 |
There was a problem hiding this comment.
4. Todo in eval.yaml 📜 Skill insight ✧ Quality
A new TODO comment was introduced in committed code/config, which is disallowed by the checklist. Leaving TODOs in committed artifacts creates ambiguous, untracked work and can mask incomplete compliance decisions.
Agent Prompt
## Issue description
A `TODO` comment was introduced in `eval/triage/eval.yaml`, which violates the prohibition on TODO/FIXME/HACK comments in committed code.
## Issue Context
The comment indicates an intended future threshold change.
## Fix Focus Areas
- eval/triage/eval.yaml[176-180]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| name: Functional Tests | ||
|
|
||
| # PR-triggered functional tests use pull_request_target so fork PRs receive | ||
| # secrets. Authorization runs in a separate gate job (base checkout only) | ||
| # before the test job checks out the PR head — same pattern as fullsend e2e. | ||
|
|
||
| permissions: {} | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| # SYNC-WITH: grep regex in "Check for functional-test-relevant changes" step | ||
| paths: | ||
| - 'eval/**' | ||
| - 'agents/**' | ||
| - 'harness/**' | ||
| - 'scripts/**' | ||
| - 'schemas/**' | ||
| - '.github/workflows/functional-tests.yml' | ||
| - '.github/scripts/**' | ||
| pull_request_target: | ||
| types: [opened, synchronize, reopened, labeled] | ||
| merge_group: | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: >- | ||
| ${{ github.event_name == 'pull_request_target' | ||
| && format('functional-{0}', github.event.pull_request.number) | ||
| || format('{0}-{1}', github.workflow, github.ref) }} | ||
| cancel-in-progress: >- | ||
| ${{ github.event_name == 'pull_request_target' | ||
| || github.ref != 'refs/heads/main' }} | ||
|
|
There was a problem hiding this comment.
5. Missing linked issue reference 📜 Skill insight § Compliance
This PR introduces substantial new CI/workflow + evaluation harness logic, but the provided PR metadata does not include a linked issue authorizing the non-trivial work. Non-trivial changes must have explicit authorization via a linked issue.
Agent Prompt
## Issue description
Non-trivial changes require an explicitly linked issue authorizing the work.
## Issue Context
This PR adds a new functional test workflow and supporting scripts/config, which is non-trivial in scope.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[1-34]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| on: | ||
| push: | ||
| branches: [main] | ||
| # SYNC-WITH: grep regex in "Check for functional-test-relevant changes" step | ||
| paths: | ||
| - 'eval/**' | ||
| - 'agents/**' | ||
| - 'harness/**' | ||
| - 'scripts/**' | ||
| - 'schemas/**' | ||
| - '.github/workflows/functional-tests.yml' | ||
| - '.github/scripts/**' | ||
| pull_request_target: | ||
| types: [opened, synchronize, reopened, labeled] | ||
| merge_group: | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: >- | ||
| ${{ github.event_name == 'pull_request_target' | ||
| && format('functional-{0}', github.event.pull_request.number) | ||
| || format('{0}-{1}', github.workflow, github.ref) }} | ||
| cancel-in-progress: >- | ||
| ${{ github.event_name == 'pull_request_target' | ||
| || github.ref != 'refs/heads/main' }} | ||
|
|
||
| jobs: | ||
| gate: | ||
| # Separate job so pull-requests: write stays out of the job that checks | ||
| # out fork head and runs tests with secrets. | ||
| # Never checkout github.event.pull_request.head.sha here. | ||
| if: >- | ||
| github.event_name == 'pull_request_target' && | ||
| (github.event.action != 'labeled' || github.event.label.name == 'ok-to-test') | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| outputs: | ||
| authorized: ${{ steps.auth.outputs.authorized }} | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| ref: ${{ github.sha }} # Base branch only — never checkout PR head in gate | ||
|
|
||
| - name: Check PR authorization | ||
| id: auth | ||
| uses: ./.github/actions/check-e2e-authorization | ||
| with: | ||
| pr_number: ${{ github.event.pull_request.number }} | ||
| repository: ${{ github.repository }} | ||
| pr_updated_at: ${{ github.event.pull_request.updated_at }} | ||
| event_action: ${{ github.event.action }} | ||
| pr_author_association: ${{ github.event.pull_request.author_association }} | ||
| pr_author_login: ${{ github.event.pull_request.user.login }} | ||
|
|
||
| functional-tests: | ||
| # For pull_request_target, runs only when gate sets authorized=true. | ||
| # Do not treat a skipped gate as authorized. | ||
| # This job checks out untrusted PR head code — no pull-requests: write here. | ||
| needs: gate | ||
| if: >- | ||
| !cancelled() && | ||
| (github.event_name != 'pull_request_target' || needs.gate.outputs.authorized == 'true') | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
6. Needs gate skips tests 🐞 Bug ≡ Correctness
In .github/workflows/functional-tests.yml, functional-tests declares needs: gate but gate only runs for pull_request_target, so for push, merge_group, and workflow_dispatch the gate job is skipped and the downstream functional test job can be skipped as well.
Agent Prompt
## Issue description
`functional-tests` has `needs: gate`, but `gate` is conditional and only executes for `pull_request_target`. When the workflow is triggered by `push`, `merge_group`, or `workflow_dispatch`, the `gate` job is skipped, which can prevent `functional-tests` from running.
## Issue Context
This workflow is explicitly configured to run on non-PR triggers (`push` to main, `merge_group`, and `workflow_dispatch`), so the pipeline should not depend on a job that never runs for those events.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[9-74]
### Suggested implementation direction
- Make `gate` run on all triggers (remove the job-level `if:`), and:
- For non-`pull_request_target` events, set `authorized=true` via a small shell step that writes to `$GITHUB_OUTPUT`.
- For `pull_request_target`, keep the existing authorization logic.
- Keep `functional-tests.needs: gate` and its existing `if:` condition so PR gating behavior remains unchanged.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| outputs: |
There was a problem hiding this comment.
7. Missing issues:write permission 🐞 Bug ⛨ Security
The gate job grants pull-requests: write but not issues: write, yet the authorization composite action deletes the ok-to-test label and creates/patches PR comments via Issues endpoints, which can cause the gate to fail or always return reason=error.
Agent Prompt
## Issue description
The authorization gate performs label deletion and sticky comment creation/updates using `repos/.../issues/...` endpoints. The `gate` job currently does not grant `issues: write`, so these API calls can 403 and break authorization behavior.
## Issue Context
The repo already demonstrates that PR label/comment operations are modeled as Issues permissions (see `fullsend.yaml` stop-fix job permissions).
## Fix Focus Areas
- .github/workflows/functional-tests.yml[45-48]
- .github/actions/check-e2e-authorization/action.yml[57-142]
- .github/scripts/check-e2e-authorization.sh[86-113]
### Suggested implementation direction
- Add `issues: write` to `jobs.gate.permissions`.
- (Optional hardening) Add `pull-requests: read` / `issues: read` as needed for the read-only endpoints used by the gate script (events listing, PR fetch), but the critical missing permission is `issues: write` for label/comment mutations.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| - name: Install yq | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| run: | | ||
| curl -sSfL "https://github.com/mikefarah/yq/releases/download/v4.47.1/yq_linux_amd64" -o /usr/local/bin/yq | ||
| chmod +x /usr/local/bin/yq | ||
|
|
||
| - name: Configure git identity | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| run: | | ||
| git config --global user.name "fullsend-eval[bot]" | ||
| git config --global user.email "fullsend-eval[bot]@users.noreply.github.com" | ||
|
|
||
| - name: Install fullsend | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| FULLSEND_VERSION="${FULLSEND_VERSION:-latest}" | ||
| if [ "$FULLSEND_VERSION" = "latest" ]; then | ||
| FULLSEND_VERSION=$(gh release view --repo fullsend-ai/fullsend --json tagName -q .tagName) | ||
| fi | ||
| VERSION_NUM="${FULLSEND_VERSION#v}" | ||
| TARBALL="fullsend_${VERSION_NUM}_linux_amd64.tar.gz" | ||
| gh release download "$FULLSEND_VERSION" \ | ||
| --repo fullsend-ai/fullsend \ | ||
| --pattern "$TARBALL" \ | ||
| --dir /tmp | ||
| tar xzf "/tmp/${TARBALL}" -C /usr/local/bin fullsend | ||
| chmod +x /usr/local/bin/fullsend | ||
| fullsend version | ||
|
|
||
| - name: Install OpenShell CLI | ||
| if: steps.changes.outputs.relevant != 'false' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| OPENSHELL_VERSION="${OPENSHELL_VERSION:-latest}" | ||
| if [ "$OPENSHELL_VERSION" = "latest" ]; then | ||
| OPENSHELL_VERSION=$(gh release view --repo NVIDIA/OpenShell --json tagName -q .tagName) | ||
| fi | ||
| gh release download "$OPENSHELL_VERSION" \ | ||
| --repo NVIDIA/OpenShell \ | ||
| --pattern "openshell-linux-amd64" \ | ||
| --dir /tmp | ||
| install -m 0755 /tmp/openshell-linux-amd64 /usr/local/bin/openshell | ||
|
|
There was a problem hiding this comment.
8. Unpinned binary downloads 🐞 Bug ⛨ Security
The workflow downloads and executes fullsend and openshell from the latest GitHub releases and downloads yq via a direct URL without checksum/signature verification, making CI runs non-reproducible and increasing supply-chain risk.
Agent Prompt
## Issue description
The workflow installs executable binaries (yq, fullsend, OpenShell) by downloading artifacts at runtime, including using `latest` tags, and does not verify integrity (checksums/signatures). This allows silent behavior changes between runs and expands the blast radius of an upstream compromise.
## Issue Context
This job later runs code with powerful credentials (`EVAL_GH_TOKEN`, GCP auth via OIDC), so minimizing unverified executable downloads is important.
## Fix Focus Areas
- .github/workflows/functional-tests.yml[139-184]
### Suggested implementation direction
- Pin `FULLSEND_VERSION` and `OPENSHELL_VERSION` to known-good tags (ideally as repo vars/inputs).
- Verify downloaded artifacts via published `sha256` files or hardcoded expected hashes.
- Prefer package-manager installs where possible (or GitHub Actions that verify checksums).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # --- Create ephemeral repo --- | ||
| uuid=$(uuidgen | tr '[:upper:]' '[:lower:]' | cut -c1-8) | ||
| CASE_ID_SAFE=$(basename "$CASE_SOURCE_DIR" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9-]/-/g') | ||
| repo_name="eval-${CASE_ID_SAFE}-${uuid}" | ||
| EPHEMERAL_REPO="${EVAL_ORG}/${repo_name}" | ||
|
|
||
| gh repo create "$EPHEMERAL_REPO" --public --description "Ephemeral eval repo (auto-deleted)" | ||
| echo "Created repo: $EPHEMERAL_REPO" |
There was a problem hiding this comment.
9. Interactive repo creation 🐞 Bug ☼ Reliability
eval/scripts/setup-fixture.sh calls gh repo create without an explicit non-interactive confirmation flag, which can prompt/fail in CI and prevent fixtures (and thus tests) from being created.
Agent Prompt
## Issue description
The functional test harness creates ephemeral repos in CI. The current `gh repo create` invocation does not include an explicit non-interactive confirmation flag, which risks a prompt or failure in a non-TTY environment.
## Issue Context
This script runs as a `before_each` hook; if it blocks/fails, the entire eval run fails.
## Fix Focus Areas
- eval/scripts/setup-fixture.sh[54-80]
### Suggested implementation direction
- Add `--confirm` to `gh repo create`.
- Optionally set `GH_PROMPT_DISABLED=1` (or equivalent) in the workflow/hook environment to ensure CI never prompts.
- Consider adding a cleanup trap to delete the repo if later steps in the hook fail after creation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewFindingsHigh
Medium
Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| # around this by rewriting dataset.path to an absolute path at runtime. | ||
| EVAL_YAML="$(mktemp "${EVAL_DIR}/${AGENT}/eval-runtime-XXXXXX.yaml")" | ||
| trap 'rm -f "$EVAL_YAML"' EXIT | ||
| yq ".dataset.path = \"${CASES_DIR}\"" "$EVAL_YAML_SRC" > "$EVAL_YAML" |
There was a problem hiding this comment.
[medium] logic-error
The existence check for EVAL_YAML_SRC on line 40 is dead code. Line 37 runs yq on EVAL_YAML_SRC which fails under set -euo pipefail if the file is missing, terminating the script before the friendly error message on line 41 is reached.
Suggested fix: Move the if [[ ! -f EVAL_YAML_SRC ]] check to before line 35 (before the mktemp/yq block).
| fi | ||
|
|
||
| if is_trusted_author "${author_association}"; then | ||
| authorized=true |
There was a problem hiding this comment.
[low] edge-case
The fallback timestamp comparison uses updated_at (which advances on any PR activity) as a proxy for last push time. In the normal workflow path, PR_UPDATED_AT is always passed from the event payload, so this only affects unexpected invocations.
| [[ -n "${TARGET_DIR:-}" && -d "${TARGET_DIR:-}" ]] && rm -rf "$TARGET_DIR" | ||
| } | ||
| trap cleanup EXIT | ||
|
|
There was a problem hiding this comment.
[low] missing-default-case
The case FIXTURE_TYPE block handles issue and pull_request but has no default case, unlike other scripts in this PR.
| branches: [main] | ||
| # SYNC-WITH: grep regex in "Check for functional-test-relevant changes" step | ||
| paths: | ||
| - 'eval/**' |
There was a problem hiding this comment.
[low] path-filter-gap
The push path filter and PR changes-detection grep regex include .github/scripts/** but omit .github/actions/**. Changes to the authorization action would not trigger or be detected by the workflow.
|
🤖 Finished Retro · ✅ Success · Started 6:19 PM UTC · Completed 6:29 PM UTC |
Retro: PR #31 — ci(eval): add functional tests for triage agentTimeline
Review quality assessmentFullsend review agent performed well overall. Key observations:
Evidence for existing issues
Autonomy readinessThe human reviewer (rh-hemartin) approved with an empty comment body, adding zero findings beyond what the bots already identified. The fullsend review agent's analysis was strictly superior to the human review on this PR. This is a signal that the review agent could be granted more blocking authority — but the recurring Proposals
Proposals filed
|
Summary
pull_request_targetauthorization gate (ok-to-test label flow)agent-eval-harnesssubmodule for workspace/execute/score pipelineKey adaptations
The tests in fullsend-ai/fullsend build fullsend from source and reference
internal/scaffold/fullsend-repoasFULLSEND_DIR. Since this repo is the scaffold directory, the adaptations are:FULLSEND_DIRdefaults to repo rootPrerequisites
The workflow requires these repo secrets/vars to be configured:
E2E_GCP_WIF_PROVIDER/E2E_GCP_SERVICE_ACCOUNT— GCP Workload Identity FederationEVAL_GH_TOKEN— GitHub token with repo + delete_repo scopeEVAL_ORG— GitHub org for ephemeral test reposEVALS_VERTEX_PROJECT_ID/EVALS_GCP_REGION/E2E_GCP_PROJECT_ID— Vertex AI configTest plan
workflow_dispatchto validate end-to-end./eval/lint-cases.sh triage🤖 Generated with Claude Code