fix(cli): thread --baseline and --show-suppressed through recursive scans (#201)#205
Open
wernerkasselman-au wants to merge 2 commits into
Open
Conversation
…cans _scan_multi_skill did not accept or forward the suppression options, and the recursive dispatch in scan() did not pass them, so scan --recursive silently ignored --baseline and --show-suppressed. Suppressed findings still counted toward each sub-skill's score and could flip the exit code to 1, breaking the incremental baseline workflow precisely in the mode meant for skill collections. Add baseline and show_suppressed parameters to _scan_multi_skill, pass them from the recursive dispatch, and forward them into the per-skill _scan_state call, mirroring the single-skill path. Add a regression test that captures the per sub-skill graph state and asserts the baseline and show_suppressed are present. Refs NVIDIA#201 Signed-off-by: Werner Kasselman <145896621+wernerkasselman-au@users.noreply.github.com>
…port The recursive summary table and combined JSON report computed finding_count as `filtered_findings or findings`, which falls through to the unfiltered findings when suppression empties filtered_findings, since an empty list is falsy. A fully baselined sub-skill then showed score 0 but a non-zero finding_count. Add _result_finding_count(), which falls back to raw findings only when filtered_findings is absent (the report node always sets it, possibly empty after suppression), and use it at both the summary table and the combined JSON report. Add a regression test asserting the combined JSON finding_count is 0 for a fully suppressed sub-skill. Surfaced by an independent review of NVIDIA#205. Refs NVIDIA#201 Signed-off-by: Werner Kasselman <145896621+wernerkasselman-au@users.noreply.github.com>
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.
What this fixes
Closes the bug I filed as #201. The suppression work (#88) and the multi-skill work (#136) did not compose: in
scan --recursive, the--baselineand--show-suppressedoptions were accepted on the command line and then silently dropped, so a baselined finding was not actually suppressed. It still counted toward each sub-skill's risk score, and it could still flip the exit code to 1. The single-skill path honoured both options, so only the recursive path regressed, which is exactly the mode you reach for to scan a whole skill collection in one pass.The change
_scan_multi_skill()did not accept the two options, the recursive dispatch inscan()did not pass them, and the per-skill_scan_state(...)call left them out. I addedbaselineandshow_suppressedparameters to_scan_multi_skill(), passed them from the recursive dispatch, and forwarded them into the per-skill_scan_state(...)call, so the recursive path now mirrors the single-skill path (cli.pyaround the single-skill_scan_statecall).Tests
Added
test_cli_recursive_forwards_baseline_and_show_suppressed. It builds a two-sub-skill collection (no root SKILL.md), generates a real baseline against one sub-skill, then captures the state handed to the graph for each sub-skill and asserts the baseline was loaded into it andshow_suppressedis set. On the old code the state carried neither key, so the test pins the regression rather than just exercising the happy path. I kept it at the plumbing level on purpose, so it does not couple to the suppression-matching internals.Refs #201