Open
Conversation
Review Summary by QodoEnable skills scanning by default with toggle support
WalkthroughsDescription• Changed --skills flag to be enabled by default • Implemented BooleanOptionalAction for toggle behavior • Added --no-skills option to disable skills scanning • Updated help text and CLI examples for clarity • Added comprehensive test coverage for skills flag behavior Diagramflowchart LR
A["--skills flag"] -- "Changed from opt-in" --> B["Enabled by default"]
B -- "Uses BooleanOptionalAction" --> C["--skills and --no-skills both work"]
C -- "Tested with" --> D["New test suite"]
File Changes1. src/agent_scan/cli.py
|
Code Review by Qodo
1. E2E tests now fail
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
knielsen404
approved these changes
May 8, 2026
![qodo-merge-etso[bot]](https://avatars.githubusercontent.com/u/12959162?s=60&v=4){kind=small}
Comment on lines
208
to
+211
| "--skills", | ||
| default=False, | ||
| action="store_true", | ||
| help="Scan skills beyond mcp servers.", | ||
| default=True, | ||
| action=argparse.BooleanOptionalAction, | ||
| help="Scan skills beyond mcp servers (default: enabled). Use --no-skills to disable.", |
There was a problem hiding this comment.
1. E2e tests now fail 🐞 Bug ≡ Correctness
--skills now defaults to True, but e2e tests still assert that skill scanning does not happen unless --skills is explicitly provided, so they will fail under the new default behavior.
Agent Prompt
### Issue description
The CLI now scans skills by default, but e2e tests still assume the old behavior (skills scanned only when `--skills` is passed). These tests will fail because they assert there are *no* skill servers when the flag is omitted.
### Issue Context
`--skills` switched from `store_true` default-off to `BooleanOptionalAction` default-on; the opt-out is now `--no-skills`.
### Fix Focus Areas
- tests/e2e/test_scan.py[147-170]
- tests/e2e/test_inspect.py[153-176]
### What to change
- For tests that verify "no skill servers", add `--no-skills` to the CLI invocation.
- Optionally add new assertions that default behavior *does* include skills, and keep `--no-skills` as the explicit opt-out path.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
2 participants
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.
No description provided.