fix(security-issue-sync): give step 2b its rules and correct the guardrails fixtures - #1340
Merged
Merged
Conversation
…drails fixtures Cases in the security-issue-sync suite were red on main, and the step-2b suite swung by up to three cases between runs of unchanged code. The swing was not model variance; each failure had a concrete cause. **Step 2b was never shown the rules it is graded against.** Its step-config extracted only `SKILL.md` section 2b, a ten-line pointer, while the signal-to-action rows it is tested on live in `signals-to-actions.md`. The model was asked to reproduce rules it could not see, so it improvised — differently each run, which is where the apparent flakiness came from. The runner already has `also_include` for steps whose text deliberately spans files; step 2b now uses it, and cases 1, 4 and 5 stop flapping. **The reporter-unresponsiveness rule carries policy, so it moves into the step body.** Even with the lookup table in the prompt, the model proposed chasing the reporter for confirmation rather than proposing to proceed. An unresponsive reporter must not block the team, per ASF security policy, so the proposal shape is now restated in section 2b rather than left in a thousand-line appendix. **Two guardrails fixtures contradicted the project's own rules.** `case-2` put an ASF CVE-tool URL in a draft reporter email, which AGENTS.md forbids, so its `clean: true` expectation was unreachable — the model had been correctly reporting a real violation for every run. `case-3` expected two violations where the step's own output spec mandates four. **The guardrails output spec contradicted AGENTS.md.** It called every bare CVE ID a violation, but AGENTS.md requires the bare ID in reporter mail before the advisory ships. The spec now carves that out, states the per-(guardrail, passage) counting rule, and pins the ordering — the list was being compared positionally while the model was free to emit it in any order. `evidence` is dropped from the case-3 assertion: the span a model quotes is its own choice, and the guardrail name plus ordering already carry the signal, so asserting it added flakiness without coverage. After this change `step-guardrails` passes 3/3 and `step-2b-proposed-changes` 7/7, both verified by re-runs. Two cases in `step-1f-process-step` remain red and are deliberately left alone, because both need a human ruling rather than a fixture tweak: - `case-7-announced-awaiting-cveorg` expects `process_step: "14"`; the model consistently answers `"14-15"`, which the step's own output spec permits as a range. Whether a tracker with the archive URL captured and the apply still outstanding is "14" or "14-15" is a question about what the process table means. - `case-5-pr-merged-awaiting-release` fails on `key_signals` list length — the model names five signals that subsume the expected three. The expected set is not wrong, but asserting a free-form list by position and length is the same fragility this change removes from `case-3`. Generated-by: Claude Opus 5
8 tasks
potiuk
added a commit
that referenced
this pull request
Sep 22, 2026
…p 1f expectations (#1341) Two of the cases left red by #1340 were the fixture disagreeing with the documented process rather than the model getting it wrong, and verifying that turned up a third problem in the runner itself. **A verdict the grader drops is no longer a failure.** `batch_grade_prose_fields` sends one rubric prompt covering every pair, and the grader intermittently omits a path from a larger batch. That pair was then recorded as `(False, "grader did not return a verdict")` — a FAIL indistinguishable in the report from a real mismatch, for a reason that has nothing to do with the candidate output. It is the mechanism behind cases that appear to flap between runs of unchanged code. The dropped subset is now re-asked once; silence that survives the retry is still reported, and a verdict the grader actively gives is never re-asked, so a NO cannot become a YES by asking twice. The structural-assertion path already drew this distinction with a tri-state verdict; the prose path now does too. **`case-7` expected the wrong step.** The process table in `gather.md` — the table this step tells the model to cross-reference — puts "Archive URL captured" at `14 → 15`, and the step's output spec says in as many words that when two consecutive rows both apply the answer is the inclusive hyphenated range. The tracker in this fixture is still open with the archive URL populated, so it sits on that row. The model answered `"14-15"` on every run; the fixture asserted `"14"`. **`case-5` asserted a field the spec calls explanatory.** `key_signals` exists so a reviewer can see why a step was chosen, and which facts are worth naming is a judgement call. The model named five signals that subsume the expected three and the case failed on the difference. Cases 1, 2 and 6 in this same step already omit the field, so 3, 4 and 5 now match the step's own majority and the spec records that it is emitted but not graded. Same fragility #1340 removed from the guardrails `evidence` field. `case-7`'s `step_description` is also shortened to the one-line shape the spec asks for; the first attempt enumerated every action in the combined apply, which made a prose-graded field fail on whichever sub-action the model left out. The verifying run is 43/45 with no dropped verdicts, against 42/45 and two dropped verdicts before. The two that remain are the grader's judgement on free text — one on whether "valid" implies "CVE-worthy", one on whether naming the confirmation step over-describes the action — and both have passed on other runs. They are left alone deliberately: rewording a fixture until a rubric happens to agree tunes the test to the model. Narrowing what the prose fields assert is the real fix, and it wants deciding on its own terms rather than inside this change. Generated-by: Claude Opus 5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
security-issue-syncsuite were red onmain, andstep-2bswung by up to three cases between runs of unchanged code. The swing was not model variance — step 2b's prompt never contained the rules it is graded against.step-2b'sstep-config.jsonextracted onlySKILL.md § 2b(a ten-line pointer) while the signal-to-action rows it is tested on live insignals-to-actions.md. It now usesalso_include, the mechanism the runner already documents for steps whose text spans files.step-guardrailsfixtures, and the step's own output spec, contradictedAGENTS.md. The model had been correctly reporting a real violation that the fixture asserted was clean.Type of change
.claude/skills/<name>/) — eval fixtures updated belowprek, workflows, validators)Test plan
prek run --all-filespasses (34 hooks, 0 failures)step-guardrails— 3/3 pass (was 1/3)step-2b-proposed-changes— 7/7 pass (was 5/7, and flapping between 4/7 and 6/7 on unchanged code)dependency-audit— 8/8, unaffecteddocs/mode-economics.mdregenerated (SKILL.mdgrew, so the measured token count moved)RFC-AI-0004 compliance
Linked issues
None — found while running the affected suites for #1326.
Notes for reviewers
The one behaviour-surface change is in
SKILL.md § 2b: the reporter-unresponsiveness proposal shape is restated in the step body instead of living only in the 1,281-line appendix. With the appendix in the prompt the model still proposed chasing the reporter rather than proposing to proceed, which inverts ASF policy that an unresponsive reporter must not block the team. Worth a careful read — this is the only part of the PR that changes what the skill tells a maintainer.Two cases are deliberately left red, because each needs a ruling rather than a fixture tweak:
step-1f/case-7-announced-awaiting-cveorgexpectsprocess_step: "14"; the model consistently answers"14-15", which the step's own output spec permits as a range. Whether a tracker with the archive URL captured and the apply outstanding is "14" or "14-15" is a question about what the process table means, not about the model.step-1f/case-5-pr-merged-awaiting-releasefails onkey_signalslength — the model names five signals that subsume the expected three. Asserting a free-form list by position and length is the same fragility this PR removes fromcase-3, but fixing it properly means deciding what that field is for.I tried a fixture fix for case-7 (it carried both
fix releasedandannounced, a half-applied flip) and reverted it: the model's answer did not change, so the hypothesis was wrong and the edit would have been an unjustified mutation.Related, not fixed here: no CI workflow runs these suites at all — no reference to
skill_evals.runnerormagpie-run-evalsunder.github/workflows/. That is how four cases drifted red unnoticed, and it sits awkwardly againstPRINCIPLES.md§9. Wiring it needs an Anthropic API key as a repo secret plus a per-run cost decision, which is a PMC call rather than something to slip into this PR.