Skip to content

ci(evals): narrow native eval trigger to paths read from the PR - #340

Merged
mrizzi merged 1 commit into
RHEcosystemAppEng:mainfrom
mrizzi:TC-6919
Oct 14, 2026
Merged

mrizzi merged 1 commit into
RHEcosystemAppEng:mainfrom
mrizzi:TC-6919

Conversation

@mrizzi

@mrizzi mrizzi commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • discover in eval-pr-run.yml: nativeRelevant now matches only plugins/sdlc-workflow/skills/triage-security/** and schemas/triage-security-(input|result).schema.json — the only paths the native run reads from the PR. Eval suite, fixtures, host policy/providers/profiles/env and host scripts come from the trusted base checkout, so changes there no longer trigger the ~18 min native run (or the eval-protected gate for untrusted PRs). Trust/approval conditions and default skill discovery are unchanged.
  • test_native_fullsend_eval_ci.py: discovery parametrization updated (3 live paths → true; eval suite, fixture, policy, provider, host script, test file → false).
  • test_fullsend_gate_eval.py: tripwire test_gate_commands_match_valid_case_tools_pin pins the Step 0.6/0.7 gate blocks of triage-security SKILL.md (GATE_COMMANDS_SHA256), parsed like valid_case_tool_observations in evals/fullsend/run.py. On change it fails explaining that native presence_gate_*/input_validation_* will read unrecognized and how to update the pin.

Test plan

  • python3 -m pytest plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py -q — 134 passed
  • python3 -m pytest plugins/sdlc-workflow/scripts/ -q — 688 passed, 2 skipped
  • Tripwire verified locally: editing the Step 0.6 or 0.7 block fails with the explanatory message (reverted)
  • uvx skillsaw (0 errors), claude plugin validate plugins/sdlc-workflow ✔

Implements TC-6919

🤖 Generated with Claude Code

Summary by Sourcery

Limit native triage-security evaluations to pull-request changes that can affect their behavior while guarding against unnoticed gate-command drift.

Enhancements:

  • Narrow native evaluation triggering to triage-security skills and input/output schemas that are read from the pull request.
  • Add a checksum tripwire to detect changes to the triage-security gate commands relied on by native evidence checks.

CI:

  • Update native evaluation discovery tests to reflect the reduced set of pull-request paths that trigger native runs.

Tests:

  • Add coverage verifying the pinned triage-security gate commands remain synchronized with native evaluation expectations.

The native Fullsend run reads only the triage-security skill folder and
the two triage-security schemas from the PR plugin; the eval suite,
fixtures, host policy/config and host scripts come from the trusted base
checkout. Triggering on those paths re-tested main and sent untrusted
PRs to the eval-protected gate for nothing. Trust/approval gating is
unchanged.

Add a pytest tripwire pinning the Step 0.6/0.7 gate commands of
triage-security SKILL.md (GATE_COMMANDS_SHA256): native valid_case_tools
diagnostics match against main's commands, so a PR changing them reads
unrecognized and needs a manual transcript check.

Implements TC-6919

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assisted-by: Claude Code
@sourcery-ai

sourcery-ai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The PR reduces native eval execution to changes in the skill and schemas actually read from the PR, leaving trusted base-checkout eval assets and host configuration changes out of the trigger path. It expands discovery coverage for both accepted and rejected paths and adds a hash-based guard to flag gate-command drift that could invalidate native evidence interpretation.

Flow diagram for narrowed native eval triggering

flowchart LR
    PR["PR changed files"] --> discover["discover"]
    discover --> relevant{"Matches triage-security skill or input/result schema?"}
    relevant -->|yes| native["Run native eval"]
    relevant -->|no| skip["Skip native eval"]
    base["Trusted base checkout\neval suite, fixtures, policy, providers, profiles, env, host scripts"] --> native
Loading

Flow diagram for gate command drift protection

flowchart LR
    skill["triage-security SKILL.md\nStep 0.6/0.7 gate blocks"] --> parse["Parse gate commands"]
    parse --> hash["Compare GATE_COMMANDS_SHA256"]
    hash -->|match| pass["Gate test passes"]
    hash -->|changed| fail["Gate test fails with update guidance"]
    fail --> evidence["Prevent unrecognized native\npresence_gate_* / input_validation_*"]
Loading

File-Level Changes

Change Details Files
Narrow native eval discovery to only PR-controlled inputs consumed by the native run.
  • Match triage-security skill files and input/result schemas.
  • Stop triggering native evals for eval suites, fixtures, host configuration, scripts, tests, and unrelated workflow changes.
  • Update discovery tests to cover the reduced trigger set and negative cases.
.github/workflows/eval-pr-run.yml
plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py
Add a tripwire for drift between trusted gate commands and native evidence parsing.
  • Hash the Step 0.6 and 0.7 gate command blocks in SKILL.md.
  • Fail with remediation guidance when the pinned commands change, since native diagnostics compare against base-checkout commands.
plugins/sdlc-workflow/scripts/test_fullsend_gate_eval.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 4 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path=".github/workflows/eval-pr-run.yml" line_range="232-234" />
<code_context>

             const skills = confirmed.join(',');
             const nativeRelevant = files.some(f =>
-              /^evals\/fullsend\//.test(f.filename) ||
-              /^evals\/triage-security\//.test(f.filename) ||
               /^plugins\/sdlc-workflow\/skills\/triage-security\//.test(f.filename) ||
-              /^plugins\/sdlc-workflow\/(policies\/triage-security\.yaml|providers\/vertex-ai\.yaml|profiles\/fullsend-vertex-ai\.yaml|env\/gcp-vertex\.env|schemas\/triage-security-(input|result)\.schema\.json|scripts\/(validate-output-schema\.sh|strip_extra_properties\.py|test_(fullsend_gate_eval|native_fullsend_eval_ci|native_fullsend_execution)\.py))$/.test(f.filename)
+              /^plugins\/sdlc-workflow\/schemas\/triage-security-(input|result)\.schema\.json$/.test(f.filename)
             );
             core.setOutput('native', String(nativeRelevant));
</code_context>
<issue_to_address>
**Renamed removals escape native evaluation**

When a triage-security skill file or schema is renamed outside the watched paths, `pulls.listFiles` reports the destination in `filename` and the old path in `previous_filename`, but discovery checks only `filename`. The rename leaves `nativeRelevant` false, so the native run skips evaluation of the removal.

Check `previous_filename` as well as `filename` when determining whether a changed file matches the watched paths.
</issue_to_address>

### Comment 2
<location path=".github/workflows/eval-pr-run.yml" line_range="234" />
<code_context>
-              /^evals\/triage-security\//.test(f.filename) ||
               /^plugins\/sdlc-workflow\/skills\/triage-security\//.test(f.filename) ||
-              /^plugins\/sdlc-workflow\/(policies\/triage-security\.yaml|providers\/vertex-ai\.yaml|profiles\/fullsend-vertex-ai\.yaml|env\/gcp-vertex\.env|schemas\/triage-security-(input|result)\.schema\.json|scripts\/(validate-output-schema\.sh|strip_extra_properties\.py|test_(fullsend_gate_eval|native_fullsend_eval_ci|native_fullsend_execution)\.py))$/.test(f.filename)
+              /^plugins\/sdlc-workflow\/schemas\/triage-security-(input|result)\.schema\.json$/.test(f.filename)
             );
             core.setOutput('native', String(nativeRelevant));
</code_context>
<issue_to_address>
**Changed plugin scripts skip native evaluation**

When a PR changes a plugin script used by the sandbox, such as `scripts/sha256-digest.py`, without changing the triage-security skill directory or either schema, `run-fullsend.py` copies the entire PR plugin into `tested-plugin`, which is supplied to the sandbox, and triage-security remediation templates invoke `scripts/sha256-digest.py`. A PR changing a plugin script outside the matched skill or schema paths therefore changes sandbox behavior without triggering the native eval, so its regression goes untested.

Include PR plugin files consumed by the sandbox in native discovery, or stage only the specific PR files the native run is intended to test.
</issue_to_address>

### Comment 3
<location path=".github/workflows/eval-pr-run.yml" line_range="234" />
<code_context>
-              /^evals\/triage-security\//.test(f.filename) ||
               /^plugins\/sdlc-workflow\/skills\/triage-security\//.test(f.filename) ||
-              /^plugins\/sdlc-workflow\/(policies\/triage-security\.yaml|providers\/vertex-ai\.yaml|profiles\/fullsend-vertex-ai\.yaml|env\/gcp-vertex\.env|schemas\/triage-security-(input|result)\.schema\.json|scripts\/(validate-output-schema\.sh|strip_extra_properties\.py|test_(fullsend_gate_eval|native_fullsend_eval_ci|native_fullsend_execution)\.py))$/.test(f.filename)
+              /^plugins\/sdlc-workflow\/schemas\/triage-security-(input|result)\.schema\.json$/.test(f.filename)
             );
             core.setOutput('native', String(nativeRelevant));
</code_context>
<issue_to_address>
**Result schema edits launch native evals**

When a PR changes only `plugins/sdlc-workflow/schemas/triage-security-result.schema.json`, the discovery regex marks a result-schema change as native-relevant, so the approval gate and native job run. `run-fullsend.py` replaces that PR schema with the trusted base version when staging, so the change cannot affect the run and still incurs the gate and runtime.

Exclude the result schema from native discovery and its expected-true test case unless the native runner is changed to consume the PR version.

Also at `plugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py:132`.
</issue_to_address>

### Comment 4
<location path=".github/workflows/eval-pr-run.yml" line_range="234" />
<code_context>
-              /^evals\/triage-security\//.test(f.filename) ||
               /^plugins\/sdlc-workflow\/skills\/triage-security\//.test(f.filename) ||
-              /^plugins\/sdlc-workflow\/(policies\/triage-security\.yaml|providers\/vertex-ai\.yaml|profiles\/fullsend-vertex-ai\.yaml|env\/gcp-vertex\.env|schemas\/triage-security-(input|result)\.schema\.json|scripts\/(validate-output-schema\.sh|strip_extra_properties\.py|test_(fullsend_gate_eval|native_fullsend_eval_ci|native_fullsend_execution)\.py))$/.test(f.filename)
+              /^plugins\/sdlc-workflow\/schemas\/triage-security-(input|result)\.schema\.json$/.test(f.filename)
             );
             core.setOutput('native', String(nativeRelevant));
</code_context>
<issue_to_address>
**Fixture edits still require approval**

When an untrusted PR changes only a file under `evals/triage-security/files/`, `evalPattern` still classifies changes under `evals/triage-security/files/` as the `triage-security` skill, and the existing `evals/triage-security/evals.json` confirms it. `skills` therefore remains nonempty, so an untrusted PR still reaches the `eval-protected` gate and ordinary eval job for these fixture-only changes.

Exclude native-only fixture changes from ordinary skill discovery, or narrow the claim to say only the native run is avoided for these paths.
</issue_to_address>

Sourcery assessment

Approval pending. 4 findings to address first.

Blocking findings: .github/workflows/eval-pr-run.yml:234, .github/workflows/eval-pr-run.yml:234, .github/workflows/eval-pr-run.yml:234, .github/workflows/eval-pr-run.yml:234


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines 232 to +234
const nativeRelevant = files.some(f =>
/^evals\/fullsend\//.test(f.filename) ||
/^evals\/triage-security\//.test(f.filename) ||
/^plugins\/sdlc-workflow\/skills\/triage-security\//.test(f.filename) ||
/^plugins\/sdlc-workflow\/(policies\/triage-security\.yaml|providers\/vertex-ai\.yaml|profiles\/fullsend-vertex-ai\.yaml|env\/gcp-vertex\.env|schemas\/triage-security-(input|result)\.schema\.json|scripts\/(validate-output-schema\.sh|strip_extra_properties\.py|test_(fullsend_gate_eval|native_fullsend_eval_ci|native_fullsend_execution)\.py))$/.test(f.filename)
/^plugins\/sdlc-workflow\/schemas\/triage-security-(input|result)\.schema\.json$/.test(f.filename)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium · Renamed removals escape native evaluation

When a triage-security skill file or schema is renamed outside the watched paths, pulls.listFiles reports the destination in filename and the old path in previous_filename, but discovery checks only filename. The rename leaves nativeRelevant false, so the native run skips evaluation of the removal.

Check previous_filename as well as filename when determining whether a changed file matches the watched paths.

Prompt for AI agents
In `.github/workflows/eval-pr-run.yml` at lines 232-234:

**Renamed removals escape native evaluation**

When a triage-security skill file or schema is renamed outside the watched paths, `pulls.listFiles` reports the destination in `filename` and the old path in `previous_filename`, but discovery checks only `filename`. The rename leaves `nativeRelevant` false, so the native run skips evaluation of the removal.

Check `previous_filename` as well as `filename` when determining whether a changed file matches the watched paths.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[sdlc-workflow/verify-pr] Classified as suggestion - valid robustness enhancement, but it describes long-standing behavior (the discovery expression has only ever matched filename, both before and after this PR) and is outside the scope of TC-6919, which narrows which paths trigger the native run rather than how renames are tracked. It is not documented in CONVENTIONS.md and no established codebase pattern uses previous_filename. No sub-task created.

Comment thread .github/workflows/eval-pr-run.yml
Comment thread .github/workflows/eval-pr-run.yml
Comment thread .github/workflows/eval-pr-run.yml

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Native Fullsend Eval Results

Head: 39c4079
Merge: 8022f1d
Trusted workflow: 66692a5
Reviewed eval source: 66692a5

Native job: failure; complete: false.
Execution evidence: invalid; quality score (advisory): unavailable (grading incomplete; 21/28 judgments available, 20 passed / 1 failed).

  • 037-release / assertion_1: scorer-error (phase-exit)
  • 037-release / assertion_2: scorer-error (phase-exit)
  • 037-release / assertion_3: scorer-error (phase-exit)
  • 037-release / assertion_4: scorer-error (phase-exit)
  • 037-release / assertion_5: scorer-error (phase-exit)
  • 037-release / assertion_6: scorer-error (phase-exit)
  • 037-release / assertion_7: scorer-error (phase-exit)
Case Execution evidence Quality score (advisory)
033-absent valid 4/4
034-empty valid 5/5
035-malformed valid 4/5
036-valid valid 7/7
037-release valid unavailable (0/7 judgments, 0 passed / 0 failed)

@mrizzi mrizzi added the ok-to-test Enable verify-pr execution on PRs from forks label Oct 14, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🤖 Finished Verify Pr · ✅ Success · Started 1:54 PM UTC · Completed 2:07 PM UTC

Commit: 39c4079 · View workflow run →

Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $6.50

@fullsend-ai-review

Copy link
Copy Markdown

Verification Report for TC-6919 (commit 39c4079)

Check Result Details
Review Feedback PASS 4 sourcery-ai suggestions; 0 code change requests; 0 sub-tasks created
Root-Cause Investigation N/A No sub-tasks created, nothing to investigate
Scope Containment PASS All 3 changed files match the task's Files to Modify exactly
Diff Size PASS 38 lines across 3 files - proportionate to task scope
Commit Traceability PASS Commit 39c4079 references TC-6919 (Implements trailer)
Sensitive Patterns PASS No secrets/credentials in added lines (SHA-256 pin is a content hash)
CI Status PASS 8/8 head-SHA checks success, incl. Script Unit Tests 3.11-3.14
Acceptance Criteria PASS 5 of 5 criteria met
Test Quality PASS Repetitive: PASS; Test docs: PASS; Eval Quality: N/A
Test Change Classification ADDITIVE +1 test fn, +4 parametrized cases, +1 assertion; no weakening
Verification Commands PASS pytest suite green on PR head via CI; regex & tripwire reproduced

Overall: PASS

All verification checks pass. The PR narrows the nativeRelevant discovery expression to exactly the three PR-read paths (triage-security skill folder + input/result schemas), updates the parametrized discovery test, and adds the GATE_COMMANDS_SHA256 tripwire - all five acceptance criteria were independently verified (discovery regex reproduced for live/dropped paths; trust/approval, eval-protected, and skills discovery byte-identical to base; tripwire digest matches current SKILL.md and fails on mutation with the explanatory message).

Review feedback: sourcery-ai raised 4 suggestions, all on eval-pr-run.yml:234. None match a CONVENTIONS.md convention or an established codebase pattern, so none were upgraded to code change requests and no sub-tasks were created. Each received a classification reply. Two are worth a human's eye because they touch deliberate task decisions: comment 4271519562 (exclude the result schema) directly conflicts with an explicit acceptance criterion that requires result-schema -> native=true; comment 4271519550 (plugin scripts consumed by the sandbox) questions the task's narrowing scope.

Advisory (non-blocking): the separate "Native Fullsend Eval Results" review reported a scorer-error (phase-exit) on case 037-release (grading incomplete, 21/28 judgments). It is not a required check-run and is infrastructure/advisory, so it does not affect CI Status or the overall verdict.

This report is informational - a human reviewer decides whether to merge.


This comment was AI-generated by sdlc-workflow/verify-pr v0.13.10.

@mrizzi
mrizzi merged commit 43f2329 into RHEcosystemAppEng:main Oct 14, 2026
22 of 23 checks passed
@mrizzi
mrizzi deleted the TC-6919 branch October 14, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Enable verify-pr execution on PRs from forks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant