Repository navigation
Conversation
…r in validate Non-strict YAML parsing silently drops unknown matcher keys, decoding a typo'd assertion (e.g. output_not_contains) to an all-zero Rule that used to pass validation. At runtime such an empty rule fails late with "unknown_rule" in success position, and no-ops in failure position, letting cases pass with zero assertions evaluated. Reject empty rules in validateJudgeTypeAndLocalFields so both `skill-up validate` and `skill-up run` (which validates selected cases before executing) refuse them with an error listing the supported matchers.
This branch has not been deployed
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
Reject
rule_basedassertions that set none of the supported matcher fields. Non-strict YAML parsing silently drops unknown matcher keys, decoding a typo'd assertion (e.g.output_not_contains) to an all-zeroRulethat previously passed validation. At runtime such an empty rule fails late withunknown_rulein success position, and no-ops in failure position, letting cases pass with zero assertions evaluated.Related issues
Closes #294
Changes
internal/config/validator.go:validateJudgeTypeAndLocalFields(shared by eval-level and case-level judges, and byrun's pre-execution validation viaValidateCasesWithEvalDefaults) now reports an error per empty rule, naming the rule position and listing all supported matchers.internal/config/validator_test.go: empty rules rejected in success/failure at both eval and case level; all 10 matchers (including the four turn-level ones) covered by false-positive-guard cases.CHANGELOG.md: entry under Unreleased / Fixed.Test plan
make testpassesmake verifypasses (fmt + vet + lint)output_not_containsnow exits 1 atskill-up validatewithjudge.success[0]: assertion has no recognizable matcher field; supported matchers: ...; the correctedoutput_contains: {not: [...]}equivalent still validates.Notes for reviewers
Zero behavior change for well-formed configs — only assertions that were already silently inert are newly refused. Strict-mode YAML decoding (
KnownFields(true)) was considered and left as a follow-up: it is a breaking change for any existing config with stray keys, while this PR only rejects rules that never did anything.