Skip to content

fix: default is --skills by def#204

Draft
mmilanta wants to merge 1 commit intomainfrom
fix/skill-by-default
Draft

fix: default is --skills by def#204
mmilanta wants to merge 1 commit intomainfrom
fix/skill-by-default

Conversation

@mmilanta
Copy link
Copy Markdown
Contributor

@mmilanta mmilanta commented Mar 5, 2026

No description provided.

Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of print_scan_inspect drops several behaviors that aren't obviously carried over into scan_with_skills: the WELL_KNOWN_MCP_PATHS fallback when args.files is empty, the stdout suppression for --json mode, and the print_scan_result call for non-JSON output. Before this is merged, it's worth confirming scan_with_skills handles all three of these paths, or this is a silent regression for users not passing --skills.

The updated tests in test_cli_parsing.py (e.g. test_json_output_suppresses_stdout_during_scan) are mechanically renaming print_scan_inspectscan_with_skills but still mock run_scan_inspect internally. If scan_with_skills no longer calls run_scan_inspect in the same way, these tests may be passing vacuously rather than validating real behavior. It would be worth checking whether the mock wiring still reflects the actual call chain inside scan_with_skills.

There's also no test covering the case where args.skills is False (or absent) and args.files is empty — previously print_scan_inspect explicitly set args.files = WELL_KNOWN_MCP_PATHS in that scenario, and that logic is now gone without a clear replacement.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants