fix(provider): isolate Claude CLI settings hooks with --setting-sources=#298
Merged
Merged
Conversation
Signed-off-by: Rod Boev <rod.boev@gmail.com>
…NVIDIA#295) Signed-off-by: Rod Boev <rod.boev@gmail.com>
keshprad
requested changes
Jul 22, 2026
keshprad
left a comment
Member
There was a problem hiding this comment.
The settings isolation is sound, but the new no-settings fallback changes the documented model-selection contract. Please update the user-facing documentation before merging.
keshprad
reviewed
Jul 22, 2026
Signed-off-by: Rod Boev <rod.boev@gmail.com>
keshprad
approved these changes
Jul 22, 2026
keshprad
left a comment
Member
There was a problem hiding this comment.
Approved after re-review: the documentation now accurately describes the CLI runtime fallback and SKILLSPECTOR_MODEL override.
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
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
The
claude_cliprovider builds a capability-strippedclaude -psubprocess, but it never constrains settings sources, so the spawned CLI loads the operator'ssettings.jsonand runs its hooks. When a hook re-enters Claude Code, each LLM call the scanner makes can turn into a full hook cycle and saturate the machine.This change adds
--setting-sources=beside the existing Claude isolation flags so the spawned CLI loads no user, project, or local settings and therefore runs none of those hooks. WithSKILLSPECTOR_MODELunset, that also means Claude no longer inherits filesystem default-model or thinking settings from those scopes.Closes #295
Reported by @Mark2Mac in issue #295.
Root cause
_build_claude_argvinsrc/skillspector/providers/_agent_cli.pyalready strips tools, MCP servers, and slash commands, but it omits the Claude CLI's settings-source boundary. The returned argv therefore still lets the child CLI load ambient settings before the model call, and settings-derived hooks execute inside what is otherwise intended to be an isolated subprocess.Before / After
claudeloads settings hooks... --strict-mcp-config --disable-slash-commands... --strict-mcp-config --setting-sources= --disable-slash-commandsDiff Notes
src/skillspector/providers/_agent_cli.py: add the exact--setting-sources=token to_build_claude_argv, directly after--strict-mcp-configsrc/skillspector/providers/_agent_cli.py: document that the empty settings-source list blocks user, project, and local settings hooks, and that filesystem model settings no longer flow into the spawned Claude CLI unless SkillSpector pinsSKILLSPECTOR_MODELsrc/skillspector/providers/_agent_cli_base.pyandsrc/skillspector/providers/claude_cli/provider.py: narrow the Claude contract text somodel=""means "no explicit override" rather than "inherit filesystem model settings"tests/unit/test_agent_cli.py: add focused argv assertions for the new literal and its required single-token empty-value formtests/unit/test_agent_cli.py: add Codex and Gemini absence checks so the preservation coverage proves--setting-sources=stays Claude-onlytests/unit/test_providers.pyandtests/integration/test_agent_cli_live.py: align the Claude-facing test language with the new no-filesystem-settings boundaryScope
This is limited to the Claude CLI subprocess boundary. It does not change Codex or Gemini builders, env scrubbing, auth behavior, graph orchestration, analyzers, report metadata, MCP transport, or dependencies.
The hook-suppression effect is owned by the external Claude CLI flag contract. The committed unit tests prove the argv SkillSpector emits at that boundary, not the external CLI's full runtime behavior. The no-settings boundary also means Claude's filesystem model settings stop applying unless
SKILLSPECTOR_MODELpins a model explicitly.Verification
.\.venv\Scripts\python.exe -m pytest tests\unit\test_agent_cli.py tests\unit\test_providers.pyreturned156 passed, 9 skipped in 2.47s.--setting-sources=; the head argv proxy emitted--strict-mcp-config,--setting-sources=, and--disable-slash-commandsin that order.uv run ruff check src/ tests/returnedAll checks passed!.uv run ruff format --check src/ tests/returned144 files already formatted.claude --helpdocumented--setting-sources <sources>foruser, project, local;claude --setting-sources= --versionreturned2.1.217 (Claude Code).33versus0log lines and1versus0created sessions for current argv versus--setting-sources=.