Repository navigation
ci(fullsend): bootstrap evals from reviewed PR299 source - #323
Conversation
Extend the existing eval approval flow with immutable PR source pins, trusted native tooling, separate host and sandbox WIF credentials, and source-bound safe reporting. Keep native execution restricted to PR299 while preserving ordinary evals and the active verify-pr rollout. Implements TC-6726 Assisted-by: Claude Code
Lock the trusted host validator dependency and preserve lexical plugin paths until root, ancestor and child symlinks are rejected. Add three regressions for fresh CI dependencies and CLI path boundaries. Implements TC-6726 Assisted-by: Claude Code
|
Sorry @mrizzi, your pull request is larger than the review limit of 150,000 diff characters |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Eval Results
Eval Summary: triage-security
Results
| Eval | Assertions | Passed | Failed | Pass Rate | Tokens | Duration |
|---|---|---|---|---|---|---|
| 1 | 11 | 11 | 0 | 100% | 64,178 | 178.1s |
| 2 | 5 | 5 | 0 | 100% | 58,422 | 96.1s |
| 3 | 5 | 5 | 0 | 100% | 45,526 | 91.6s |
| 4 | 5 | 5 | 0 | 100% | 60,646 | 123.9s |
| 5 | 6 | 6 | 0 | 100% | 53,058 | 117.2s |
| 6 | 6 | 6 | 0 | 100% | 36,194 | 83.2s |
| 7 | 5 | 5 | 0 | 100% | 45,550 | 93.4s |
| 8 | 8 | 8 | 0 | 100% | 47,922 | 113.5s |
| 9 | 5 | 5 | 0 | 100% | 45,762 | 90.8s |
| 10 | 5 | 5 | 0 | 100% | 55,139 | 147.3s |
| 11 | 5 | 5 | 0 | 100% | 46,019 | 95.7s |
| 12 | 5 | 5 | 0 | 100% | 53,587 | 206.3s |
| 13 | 5 | 5 | 0 | 100% | 55,782 | 254.4s |
| 14 | 5 | 5 | 0 | 100% | 48,633 | 129.5s |
| 15 | 5 | 5 | 0 | 100% | 60,537 | 218.3s |
| 16 | 7 | 7 | 0 | 100% | 35,909 | 71.3s |
| 17 | 5 | 5 | 0 | 100% | 41,889 | 140.9s |
| 18 | 5 | 5 | 0 | 100% | 47,977 | 135.1s |
| 19 | 5 | 5 | 0 | 100% | 35,529 | 50.6s |
| 20 | 4 | 4 | 0 | 100% | 36,528 | 60.9s |
| 21 | 4 | 4 | 0 | 100% | 40,296 | 107.9s |
| 22 | 4 | 4 | 0 | 100% | 47,288 | 125.9s |
| 23 | 4 | 4 | 0 | 100% | 46,277 | 146.8s |
| 24 | 4 | 4 | 0 | 100% | 45,239 | 119.1s |
| 25 | 4 | 4 | 0 | 100% | 36,365 | 56.6s |
| 26 | 5 | 5 | 0 | 100% | 52,413 | 118.6s |
| 27 | 5 | 5 | 0 | 100% | 46,771 | 99.6s |
| 28 | 4 | 4 | 0 | 100% | 55,609 | 157.1s |
| 29 | 5 | 5 | 0 | 100% | 44,833 | 49.0s |
| 30 | 4 | 4 | 0 | 100% | 43,987 | 78.4s |
| 31 | 4 | 4 | 0 | 100% | 44,484 | 63.4s |
| 32 | 4 | 4 | 0 | 100% | 43,146 | 63.4s |
Aggregate
| Metric | Mean | Stddev |
|---|---|---|
| Pass Rate | 100.0% | 0.0% |
| Duration | 115.1s | 48.4s |
| Tokens | 47,547 | 7,557 |
Total: 32 evals, 163 assertions, 163 passed, 0 failed
Baseline Comparison
No baseline found at evals/triage-security/baselines/latest/. This is the first recorded run.
Generated by sdlc-workflow v0.13.9
Remove all native suite, fixture, dependency, companion and documentation files from the main bootstrap. Keep only the two workflows, setup wrapper and workflow contracts. Fetch the explicitly reviewed PR299 suite commit and verify it before host execution; bind suite provenance to reports. Implements TC-6726 Assisted-by: Claude Code
mrizzi
changed the title
Reviewer's GuideIntroduces a narrowly scoped, PR299-only native Fullsend CI bootstrap that executes a reviewed, immutable suite against an immutable PR merge with approval and WIF safeguards, publishes only validated source-bound evidence, and verifies the workflow contracts with synthetic tests. Sequence diagram for the PR299 native Fullsend eval runsequenceDiagram
participant Workflow as GitHub Actions
participant Discover as Discover job
participant Gate as Approval gate
participant Native as Native eval job
participant Suite as Reviewed eval source
participant Plugin as PR merge sandbox
participant Report as Safe result reporter
Workflow->>Discover: Validate PR identity and merge_sha
Discover->>Discover: Set native output for PR299 and verify-pr-fullsend
Discover->>Gate: Request eval-protected approval
Gate-->>Native: Approval succeeds
Native->>Suite: Checkout NATIVE_EVAL_SOURCE_SHA
Native->>Plugin: Checkout exact merge_sha
Native->>Native: run-native-fullsend-evals.sh setup
Native->>Native: run-native-fullsend-evals.sh run
Native->>Suite: run.py run against plugin-root
Suite-->>Native: Allowlisted native-result.json
Native->>Report: Upload source-bound result
Report->>Report: Validate revisions, completeness, and 21 assertions
Report-->>Workflow: Publish review and commit status
Flow diagram for native Fullsend activation and reportingflowchart TD
A[PR workflow triggered] --> B[Discover PR context]
B --> C{PR299 targeting main from verify-pr-fullsend?}
C -- No --> D[Run ordinary eval path]
C -- Yes --> E{Native-relevant files changed?}
E -- No --> D
E -- Yes --> F[Approval gate for exact head and merge]
F --> G[Checkout trusted base, exact PR merge, reviewed suite pin, and pinned Fullsend]
G --> H[Recheck approved revision before WIF]
H --> I[Authenticate host with existing WIF]
I --> J[Run native suite in sandbox with separate credentials]
J --> K[Upload only allowlisted result]
K --> L{Source binding and 21 assertions valid?}
L -- Yes --> M[Publish native review and success status]
L -- No --> N[Publish failure evidence and fail status]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 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="402-404" />
<code_context>
+ (needs.discover.outputs.trusted == 'true' || needs.gate.result == 'success')
+ runs-on: ubuntu-24.04
+ timeout-minutes: 90
+ permissions:
+ contents: read
+ id-token: write
+ steps:
+ - name: Checkout trusted base
</code_context>
<issue_to_address>
**Revision check lacks API permission**
When a native eval job reaches the revision-recheck step, the job-level permissions replace the workflow-level permissions, leaving this job without `pull-requests: read`. The `Recheck approved revision before WIF` step calls `github.rest.pulls.get`, which GitHub rejects for lack of permission; every requested native run fails before WIF authentication.
Add `pull-requests: read` to the native job's permissions.
</issue_to_address>
### Comment 2
<location path=".github/workflows/eval-pr-run.yml" line_range="576" />
<code_context>
+ process.env.NATIVE_REPORT_RESULT === 'success');
+ const ordinaryOk = !ordinaryRequested || evalsResult === 'success';
+ const state = discoverResult === 'success' && gateResult !== 'failure' && gateResult !== 'cancelled' &&
+ nativeOk && ordinaryOk ? 'success' : 'failure';
+ const description = state === 'success' ? 'Requested eval suites completed successfully' :
+ 'Eval execution, approval or native evidence failed';
</code_context>
<issue_to_address>
**Older runs overwrite current results**
When two workflow runs for the same PR head overlap and resolve different merge/base revisions, `report-status` and the ordinary review update publish by head SHA without checking whether a newer run has reported. An older run that finishes last overwrites the newer run’s status and review with results for its older merge, so callers see stale results as current.
Before publishing either a status or an ordinary review update, verify that the run is still the latest for that PR head.
Also at `.github/workflows/eval-pr-run.yml:370`, `.github/workflows/eval-pr-run.yml:577`.
</issue_to_address>
### Comment 3
<location path=".github/workflows/eval-pr-run.yml" line_range="550" />
<code_context>
+ // Only constructed scalar/count data enters the review. No arbitrary
+ // rationale, transcript, credential path or PR-controlled Markdown.
+ await github.rest.pulls.createReview({...context.repo, pull_number: expected.pr_number,
+ commit_id: expected.head_sha, event: 'COMMENT', body});
+ if (!valid) core.setFailed('Native evidence incomplete, failed, or missing');
+
</code_context>
<issue_to_address>
**Duplicate reviews accumulate**
When the native workflow is rerun for the same head, or concurrent ordinary eval runs for the same head reach the review check, the native publisher calls `createReview` on every run, so reruns create duplicate native result reviews. The ordinary publisher’s find-then-create sequence is not atomic, so concurrent runs can also create duplicate reviews, leaving repeated comments on the PR.
Make native review publication reuse or update an existing result review, and make ordinary review creation atomic or otherwise safe under concurrent runs.
Also at `.github/workflows/eval-pr-run.yml:370`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and this adds a credentialed CI trust boundary that runs PR-controlled plugin code alongside cloud WIF credentials and external native inference. If the source pinning, sandboxing, or credential isolation is wrong, a malicious or flawed revision could expose credentials or cause unbounded cloud-side effects; reverting the workflow would not undo those effects.
Blocking findings: .github/workflows/eval-pr-run.yml:404, .github/workflows/eval-pr-run.yml:576, .github/workflows/eval-pr-run.yml:550
Code review (high effort)Security-hardened CI bootstrap PR — trust/credential flow checks out clean (native eval only activates for the real collaborator PR #299, the credentialed job re-checks the approved revision before WIF auth, final status fails closed). No exploitable false-pass or credential-leak path found in the diff. Findings skew to maintainability/robustness, most severe first: 1. Three sources of truth for the native-path regex — 2. 3. Transient 4. Native pr-head checkout omits 5. Credential-parsing loop is stricter than GITHUB_ENV guarantees — 6. Native activation hardcoded to 🤖 Generated with Claude Code |
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai review — Classified as code change request (3 findings). This review body restates the three inline findings already tracked:
No additional sub-tasks created (the findings are the same as the inline comments). |
Verification Report for TC-6726 (commit 4cd3c96)
Overall: WARNThe PR is a clean, well-scoped four-file CI bootstrap: delivery boundary respected, no secrets, deny-by-default WIF, trusted/sandbox credential separation, read-only native job (plus OIDC), all local checks green. Three sourcery-ai findings on
Acceptance Criteria is WARN because the keystone criterion — a real WIF-backed native CI run proving execution + 21-judgment grading — is explicitly deferred to a post-merge human bootstrap per the task's own delivery sequence, so it is only statically verifiable today. This report does not merge the PR or transition the issue; a human reviewer decides next steps. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
Implements TC-6729 Assisted-by: Claude Code
Implements TC-6730 Assisted-by: Claude Code
Implements TC-6731 Assisted-by: Claude Code
mrizzi
left a comment
mrizzi
left a comment
There was a problem hiding this comment.
Follow-up on 3 robustness findings from my earlier high-effort review that the post-review fix commits didn't cover (those addressed the Sourcery findings). Inline below.
|
[sdlc-workflow/verify-pr] Re: @mrizzi review — Classified as code change request (3 findings). The three robustness follow-ups are tracked as sub-tasks of TC-6726: TC-6740 (poll/retry transient null |
Verification Report for TC-6726 (commit bbcf960)
Overall: WARNThe four-file native-eval CI bootstrap is sound: scope is exactly the intended CI files (native suite/fixtures/companions/docs correctly reserved for PR299), all deterministic tests/lint/plugin checks pass, CI is green, the ordinary eval suite is 163/163, and the trust/credential/pin flow (WIF separation, PR-source pinning, read-only GitHub permissions, source-bound safe reporting) checks out with no secrets in the diff. Two WARNs, neither a FAIL:
This report does not merge the PR or transition the Jira issue — a human reviewer decides. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
Implements TC-6740 Assisted-by: Claude Code
Implements TC-6741 Assisted-by: Claude Code
Implements TC-6742 Assisted-by: Claude Code
mrizzi
left a comment
mrizzi
left a comment
There was a problem hiding this comment.
Focused high-effort re-review of the 6 post-review fix commits (4cd3c960..7ded9d6f), concentrating on the newly-added supersession guard, concurrency, and credential parser. 5 findings inline, most severe first. The heredoc parser itself is well-structured and well-tested — findings against it are the per-line masking gap and a narrowed fail-closed behavior.
|
[sdlc-workflow/verify-pr] Re: @mrizzi review — Classified the 5 inline findings from your focused re-review of
and 1 suggestion — superseded-run compute optimization ( |
Verification Report for TC-6726 (commit 7ded9d6)
Overall: WARNNo blocking (FAIL) findings. The 4-file CI bootstrap is scope-clean, CI is green, no secrets, triage-security evals pass 163/163, and all PR323-scoped acceptance criteria are satisfied (those whose proof lives on PR299 or the hosted WIF run are verified indirectly). This re-review surfaced 5 new reviewer findings on the on-branch fix commits ( This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
Implements TC-6744 Assisted-by: Claude Code
Implements TC-6745 Assisted-by: Claude Code
Implements TC-6746 Assisted-by: Claude Code
Implements TC-6747 Assisted-by: Claude Code
Verification Report for TC-6726 (commit 4da8bdd)
Overall: WARNNo new actionable items were created this run — all review feedback is already tracked as sub-tasks (TC-6729…6747), CI is green on this head, and the triage-security eval run passed 163/163. The three WARNs are informational and benign:
This PR is a minimal 4-file CI bootstrap; a human merge followed by a real WIF-backed native eval run on PR299 remains the final verification gate. This report does not merge the PR or transition the issue. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
1 participant
Summary
Implements TC-6726 for TC-6201.
Corrected scope: this PR is only the minimal CI bootstrap. All native suite files are delivered by PR #299. The earlier oversized commits are corrected by 4cd3c96; the final diff against main contains exactly these four files:
.github/workflows/eval-pr.yml.github/workflows/eval-pr-run.yml.github/scripts/run-native-fullsend-evals.shplugins/sdlc-workflow/scripts/test_native_fullsend_eval_ci.py— workflow-only contractsThere are no native cases, fixtures, judges, suite/dependency files, production companions or suite documentation in this PR's final diff.
Implements TC-6729: grant the native revision recheck read-only PR metadata access.
Implements TC-6730: check the latest API-identified workflow run/attempt before status or ordinary/native review publication; refuse publication when superseded or unverifiable.
Implements TC-6731: serialize runs for the same PR head and paginate/reuse matching bot reviews for both suites, preventing duplicate publication.
Review-fix verification: 93 repository tests passed, including 90 required scripts tests; Skillsaw 0 errors / 7 existing warnings, plugin validation and diff checks passed. Independent review found no concrete blocker. Each sub-task has one commit. Hosted WIF execution remains to be validated after bootstrap merges.
Implements TC-6744: conclude the commit status with failure when the publication guard cannot determine whether the run is current.
Implements TC-6745: tolerate delayed indexing of the current run while still suppressing older runs and attempts.
Implements TC-6746: explicitly register escaped masks for each nonempty credential line. The current GitHub runner already splits multiline masks; this makes registration explicit.
Implements TC-6747: fail before native inference on unexpected credential names without printing their values, preserving valid blank-line and heredoc parsing.
Validation for these four sub-tasks: 113 repository tests and 110 script tests passed; Skillsaw, plugin validation, Bash syntax, and whitespace checks passed. One commit per sub-task, pushed together once. Hosted native Fullsend/WIF validation remains pending.
Execution and trust
The bootstrap activates native evals only for PR #299 targeting main from verify-pr-fullsend. It checks out the reviewed suite at immutable b46b2bf647be8451b6aedda58dc8e51448f0eab6, verifies that checkout before installing or executing its code, and separately checks out the exact approved PR merge as sandbox plugin input.
This retains existing collaborator/environment approvals, immutable head/base/merge checks before WIF, existing Fullsend WIF settings and upstream setup, separate host/judge and sandbox ADC, and safe source-bound reporting. Native execution has readonly GitHub permissions plus OIDC; reporting writes remain separate. Arbitrary PR scripts/locks/validators do not execute on the credentialed host. Reports bind the tested revision, trusted workflow and reviewed suite source. Ordinary manifests and main's verify-pr dispatch are unchanged.
Verification
python3 -m pytest -q: 71 passed.Merge order
github.shaas suite source for future relevant PRs.No native suite lands on main until PR #299 merges. No agent merges either PR. Production mutation/native verify-pr coverage remains outside this suite; TC-6201 is not complete from the bootstrap alone.
Summary by Sourcery
Bootstrap source-bound native Fullsend evaluations in CI with fail-closed publication and trusted execution safeguards.
New Features:
Bug Fixes:
Enhancements:
CI:
Tests: