Skip to content

[BUG] Recursive multi-skill scans silently ignore --baseline and --show-suppressed #201

Closed

Description

@wernerkasselman-au

What I'm seeing

I was checking how the suppression work (#88) and the multi-skill work (#136) interact, and they do not: in scan --recursive, the --baseline and --show-suppressed options are accepted on the command line and then silently dropped, so a baselined finding is not actually suppressed. It still counts toward each sub-skill's risk score, and it can still flip the exit code to 1. The single-skill path honours both options, so only the recursive path regresses, which makes it easy to miss.

Walking the code (commit 7bc9c0f, src/skillspector/cli.py)

  • The recursive dispatch hands over neither option (cli.py:284):
    _scan_multi_skill(detection, format, output, no_llm, yara_rules_dir, verbose)
  • _scan_multi_skill() has no baseline / show_suppressed parameters at all (cli.py:359-366), and its per-skill state build leaves them out (cli.py:379):
    state = _scan_state(str(skill.path), format, no_llm, yara_rules_dir=yara_dir)
  • The single-skill path, for contrast, passes both through (cli.py:302-309).
  • And _scan_state() only loads the baseline and sets show_suppressed when baseline is actually provided (cli.py:142-145), so omitting the arguments means no suppression is applied, not "default suppression".

Reproduction

# A collection dir with at least two sub-skills (each with its own SKILL.md),
# plus a baseline generated from a prior scan of that collection.
skillspector scan ./skill-collection/ --recursive \
    --baseline .skillspector-baseline.yaml --show-suppressed
  • What I get: every finding in the baseline is still reported and still counted in each sub-skill's score, --show-suppressed does nothing, and a fully baselined collection can still exit 1 (max_score > 50).
  • What I expected: baselined findings suppressed per sub-skill the same way single-skill mode does it, kept out of the score, and listed only when --show-suppressed is set.

Why it matters

It is silent (no warning that the flags were ignored), so the natural assumption is that suppression is being applied. That breaks the incremental, "only the new findings matter" workflow #88 was built for, and it breaks it precisely on --recursive, which is the mode you would reach for to scan a whole skill registry in one pass.

What I think the fix is

Thread the two options through: add baseline: Path | None and show_suppressed: bool to _scan_multi_skill(), pass them from the call site at cli.py:284, and forward them into the _scan_state(...) call at cli.py:379, mirroring cli.py:302-309. Happy to put that up with a test that runs a baselined collection through --recursive and asserts the suppressed findings stay out of the per-skill scores.

Related

Activity

  1. rng1995 commented on Aug 14, 2026

    @rng1995
    Collaborator

    Active implementation: PR #205 carries the recursive baseline/show-suppressed fix. PR #375 is a related reporting/counting follow-up that also references this issue. Keeping the issue open until the primary fix merges.

  2. wernerkasselman-au commented on Aug 15, 2026

    @wernerkasselman-au
    ContributorAuthor

    @rng1995 the framing above needs a second look, and the timing is on me: I left
    a longer note on #205 (#205 (comment))
    a couple of hours after you wrote this, so you would not have seen it yet.

    The short of it is that 34f6030 (#308) made recursive plus --baseline an
    explicit exit 2 and dropped both options from _scan_multi_skill() entirely, so
    #205 in its current shape would re-enable a feature main deliberately removed,
    which is why I have not simply rebased it. Whether you want that reversed, or
    want the fingerprint scoping fixed first (fingerprints hash a path relative to
    each sub-skill root, so a finding at the same relative path in a sibling
    sub-skill gets suppressed too), is the open question on that thread.

    Two things that may help triage. This issue's title is now stale, since the
    options are no longer silently ignored but rejected with a documented exit 2.
    And #375 does not depend on the answer either way, so it can land independently
    of however #205 resolves.

    Werner

  3. rng1995 commented on Aug 20, 2026

    @rng1995
    Collaborator

    Closing this issue because the reported silent-drop behavior no longer exists on current main. Recursive multi-skill scans now reject --baseline explicitly, print an actionable error, and exit with code 2 rather than accepting the option without applying suppression.

    We are keeping that fail-fast contract for now. Safely supporting recursive baselines requires sub-skill-scoped fingerprint and path matching so a finding in one sub-skill cannot inadvertently suppress a matching finding in a sibling. That work should be tracked separately if we choose to add the feature.

    PR #375 is independent and addresses finding selection/counting only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions