fix(mcp): remove ignored tool parameters (#528)

CHANGELOG.md

@@ -39,6 +39,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Removed
 
+- **BREAKING:** `audit_openssf_baseline` no longer accepts `auto_init_config`,
+  and `get_pending_data` no longer accepts `level` (#528). Both were ignored:
+  the audit never created a project file, and pending questions were never
+  filtered by maturity level. Create an empty `.project/darnit.yaml` with
+  `init_project_config`. `get_pending_context` no longer accepts `level`;
+  that ignored parameter was removed in #552.
 - `.baseline.toml` reading and the code that existed only for it:
   `load_user_config` (including its `trusted` path),
   `load_user_config_with_report`, `validate_user_config`, `deep_merge`, and

packages/darnit-baseline/src/darnit_baseline/tools.py

@@ -93,7 +93,6 @@ def audit_openssf_baseline(
     level: int = 3,
     tags: str | list[str] | None = None,
     output_format: str = "markdown",
-    auto_init_config: bool = True,
     attest: bool = False,
     sign_attestation: bool = True,
     staging: bool = False,
@@ -121,7 +120,6 @@ def audit_openssf_baseline(
               Default: "markdown".
               Use "summary" for compact JSON (id/status/level/details only, no evidence).
               Use "badge" to generate an OpenSSF Best Practices Badge automation-proposal URL.
-        auto_init_config: Create .project.yaml if missing. Default: True
         attest: Generate in-toto attestation after audit. Default: False
         sign_attestation: Sign attestation with Sigstore. Default: True
         staging: Use Sigstore staging environment. Default: False
@@ -764,7 +762,6 @@ def _context_key_order() -> list[str]:
 def get_pending_data(
     local_path: str = ".",
     control_ids: list[str] | None = None,
-    level: int = 3,
     owner: str | None = None,
     repo: str | None = None,
     limit: int = 4,
@@ -787,7 +784,6 @@ def get_pending_data(
     Parameters:
     - `local_path`: Path to repository (default: ".")
     - `control_ids`: Optional list of control IDs to check
-    - `level`: Maximum maturity level (1, 2, or 3)
     - `owner`: GitHub owner (auto-detected if not provided)
     - `repo`: GitHub repo name (auto-detected if not provided)
     - `limit`: Max questions to return per batch (default: 4). Use 0 for all.

packages/darnit/src/darnit/skills/darnit-data/SKILL.md

@@ -13,7 +13,7 @@ Guide the user through providing missing project data that darnit needs for accu
 
 ## Workflow
 
-1. Call `get_pending_data` MCP tool with `local_path` set to the repository root and `level` set to `3`.
+1. Call `get_pending_data` MCP tool with `local_path` set to the repository root.
    - If the user mentioned a profile, pass it as the `profile` parameter to scope questions to that profile's controls.
    - This call writes nothing.
 

tests/darnit/parity/tier1/conftest.py

@@ -138,7 +138,6 @@ def mcp_tool_result(prepared_fixture: Path) -> AuditResult:
         local_path=str(prepared_fixture),
         level=3,
         output_format="json",
-        auto_init_config=False,
         attest=False,
         prefer_upstream=False,
     )

tests/darnit/parity/tier1/test_no_product_changes.py

@@ -13,6 +13,7 @@
 from __future__ import annotations
 
 import os
+import re
 import subprocess
 
 import pytest
@@ -75,6 +76,30 @@ def _base_ref() -> str | None:
     return _find_reachable_base()
 
 
+# Issue #528 dropped the ignored ``auto_init_config`` parameter. The parity
+# callers had to stop passing it. Those mechanical edits are not a parity
+# feature. A harness file qualifies only when every changed line is one of
+# the lines below, and the same change removes the parameter from the
+# product signature. Any other added or removed line keeps the file in scope.
+_AUTO_INIT_CONFIG_REMOVED_LINES = frozenset(
+    {
+        "auto_init_config=False,",
+        'args["auto_init_config"] = False',
+        "# Force safe defaults.",
+    }
+)
+_AUTO_INIT_CONFIG_ADDED_LINES = frozenset(
+    {
+        'args.pop("auto_init_config", None)',
+        "# Force safe defaults. Drop the removed no-op if a model still sends it.",
+    }
+)
+_AUTO_INIT_CONFIG_SIGNATURE = re.compile(r"^auto_init_config\s*:\s*bool\b")
+_SELF = "tests/darnit/parity/tier1/test_no_product_changes.py"
+_FIXTURES = "tests/darnit/parity/fixtures/"
+_PRODUCT_PREFIXES = ("packages/darnit/src/", "packages/darnit-baseline/src/")
+
+
 def _is_ci() -> bool:
     """True on GitHub Actions and most other CI runners.
 
@@ -85,6 +110,79 @@ def _is_ci() -> bool:
     return bool(os.environ.get("CI"))
 
 
+def _diff_line_bodies(diff_text: str) -> tuple[list[str], list[str]]:
+    """Added and removed line bodies from a unified diff, headers excluded."""
+    removed: list[str] = []
+    added: list[str] = []
+    for raw in diff_text.splitlines():
+        if raw.startswith(("diff ", "index ", "---", "+++", "@@", "\\")):
+            continue
+        if raw.startswith("-"):
+            removed.append(raw[1:].strip())
+        elif raw.startswith("+"):
+            added.append(raw[1:].strip())
+    return removed, added
+
+
+def _is_auto_init_config_callsite_diff(diff_text: str) -> bool:
+    """True when the diff only drops or pops the obsolete ``auto_init_config`` argument."""
+    removed, added = _diff_line_bodies(diff_text)
+    if any(line not in _AUTO_INIT_CONFIG_REMOVED_LINES for line in removed):
+        return False
+    if any(line not in _AUTO_INIT_CONFIG_ADDED_LINES for line in added):
+        return False
+    removed_code = [line for line in removed if not line.startswith("#")]
+    added_code = [line for line in added if not line.startswith("#")]
+    if not removed_code:
+        return False
+    assignments = [line for line in removed_code if line == 'args["auto_init_config"] = False']
+    kwargs = [line for line in removed_code if line == "auto_init_config=False,"]
+    pops = [line for line in added_code if line == 'args.pop("auto_init_config", None)']
+    return len(assignments) + len(kwargs) == len(removed_code) and pops == added_code and len(pops) == len(assignments)
+
+
+def _product_diff_removes_auto_init_config_parameter(product_diff: str) -> bool:
+    """True when a removed line is the ``auto_init_config`` signature parameter."""
+    for raw in product_diff.splitlines():
+        if raw.startswith("-") and not raw.startswith("---") and _AUTO_INIT_CONFIG_SIGNATURE.match(raw[1:].strip()):
+            return True
+    return False
+
+
+def _harness_candidates(changed: list[str]) -> list[str]:
+    return [
+        path
+        for path in changed
+        if path.startswith("tests/darnit/parity/") and not path.startswith(_FIXTURES) and path != _SELF
+    ]
+
+
+def parity_harness_feature_paths(
+    changed: list[str],
+    harness_diffs: dict[str, str],
+    product_diff: str,
+) -> list[str]:
+    """Parity paths that make this a feature-028 PR.
+
+    Call-site updates for ``auto_init_config`` drop out only when the product
+    diff removes that parameter. A missing harness diff is not compatibility.
+    """
+    candidates = _harness_candidates(changed)
+    if not candidates or not _product_diff_removes_auto_init_config_parameter(product_diff):
+        return candidates
+    return [path for path in candidates if not _is_auto_init_config_callsite_diff(harness_diffs.get(path, ""))]
+
+
+def _git_diff(base: str, *paths: str) -> str:
+    rc = subprocess.run(
+        ["git", "diff", "--unified=0", f"{base}...HEAD", "--", *paths],
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    return rc.stdout
+
+
 def test_no_product_source_changes() -> None:
     """FR-014: parity-tests PR MUST NOT modify product source. Test/config
     files are exempt so a build-config touch or a test refactor stays in
@@ -130,26 +228,110 @@ def test_no_product_source_changes() -> None:
     # Exclude this file itself from the heuristic: a PR that only touches
     # the guard (to tune scope, adjust base-ref detection, etc.) is a
     # meta-change to the guard, not a parity-tests-feature PR.
-    _SELF = "tests/darnit/parity/tier1/test_no_product_changes.py"
-    _FIXTURES = "tests/darnit/parity/fixtures/"
-    touches_parity_harness = any(
-        f.startswith("tests/darnit/parity/") and not f.startswith(_FIXTURES) and f != _SELF for f in changed
-    )
-    if not touches_parity_harness:
+    #
+    # Issue #528 also updates existing audit call sites so they no longer
+    # pass ``auto_init_config``. That is a compatibility edit, not a parity
+    # feature, when the product signature removes the parameter in this
+    # same change. The whole file stays in scope if any other line changes.
+    candidates = _harness_candidates(changed)
+    product_paths = [path for path in changed if path.startswith(_PRODUCT_PREFIXES)]
+    product_diff = _git_diff(base, *product_paths) if product_paths else ""
+    harness_diffs = {path: _git_diff(base, path) for path in candidates}
+    feature_paths = parity_harness_feature_paths(changed, harness_diffs, product_diff)
+    if not feature_paths:
+        if candidates:
+            pytest.skip(
+                "FR-014 check skipped: parity-tree edits only drop the removed "
+                "auto_init_config call sites, and the product signature drops "
+                "that parameter in this change.",
+            )
         pytest.skip(
             "FR-014 check skipped: PR does not modify the parity harness "
             "(tests/darnit/parity/ outside fixtures/, other than this guard), "
             "so it is not a parity-tests PR and FR-014's product-code "
             "guardrail does not apply.",
         )
 
-    forbidden = [
-        f for f in changed if (f.startswith("packages/darnit/src/") or f.startswith("packages/darnit-baseline/src/"))
-    ]
+    forbidden = [path for path in changed if path.startswith(_PRODUCT_PREFIXES)]
     assert not forbidden, (
         "Feature 028 (parity tests) MUST NOT modify product source code "
         "(FR-014). Offending files:\n"
         + "\n".join(f"  - {f}" for f in forbidden)
         + "\n\nIf a change under packages/*/src/ is genuinely necessary, "
         "split it into a separate PR that is not scoped to feature 028."
     )
+
+
+_PRODUCT = "packages/darnit-baseline/src/darnit_baseline/tools.py"
+_SKILL = "packages/darnit/src/darnit/skills/darnit-data/SKILL.md"
+_FIXTURE = "tests/darnit/parity/fixtures/all_pass_repo/parity.toml"
+_CONFTEST = "tests/darnit/parity/tier1/conftest.py"
+_RUN = "tests/darnit/parity/tier2/run.py"
+_OPENAI = "tests/darnit/parity/tier2/backends/openai_backend.py"
+
+_SIGNATURE_DIFF = """\
+--- a/packages/darnit-baseline/src/darnit_baseline/tools.py
++++ b/packages/darnit-baseline/src/darnit_baseline/tools.py
+@@ -96,1 +95,0 @@ def audit_openssf_baseline(
+-    auto_init_config: bool = True,
+"""
+
+_KWARG_DIFF = """\
+--- a/tests/darnit/parity/tier1/conftest.py
++++ b/tests/darnit/parity/tier1/conftest.py
+@@ -141,1 +140,0 @@ def mcp_tool_result(
+-        auto_init_config=False,
+"""
+
+_POP_DIFF = """\
+--- a/tests/darnit/parity/tier2/backends/openai_backend.py
++++ b/tests/darnit/parity/tier2/backends/openai_backend.py
+@@ -83,2 +83,2 @@ def _dispatch_tool_call(
+-        # Force safe defaults.
+-        args["auto_init_config"] = False
++        # Force safe defaults. Drop the removed no-op if a model still sends it.
++        args.pop("auto_init_config", None)
+"""
+
+
+def test_528_compatibility_edits_are_not_a_parity_feature() -> None:
+    """The three #528 call-site updates do not make this a parity-tests PR."""
+    changed = [_PRODUCT, _SKILL, _CONFTEST, _RUN, _OPENAI]
+    diffs = {_CONFTEST: _KWARG_DIFF, _RUN: _KWARG_DIFF, _OPENAI: _POP_DIFF}
+
+    assert parity_harness_feature_paths(changed, diffs, _SIGNATURE_DIFF) == []
+
+
+def test_harness_logic_change_with_product_edits_stays_in_scope() -> None:
+    """An extra assertion beside the call-site edit is still a parity-feature change."""
+    tainted = _KWARG_DIFF + "+        assert report.is_green\n"
+    changed = [_PRODUCT, _CONFTEST]
+
+    assert parity_harness_feature_paths(changed, {_CONFTEST: tainted}, _SIGNATURE_DIFF) == [_CONFTEST]
+
+
+def test_product_only_change_is_not_a_parity_feature() -> None:
+    assert parity_harness_feature_paths([_PRODUCT, _SKILL], {}, _SIGNATURE_DIFF) == []
+
+
+def test_fixture_only_change_is_not_a_parity_feature() -> None:
+    changed = [_PRODUCT, _FIXTURE]
+
+    assert parity_harness_feature_paths(changed, {}, _SIGNATURE_DIFF) == []
+
+
+def test_compatibility_without_signature_removal_stays_in_scope() -> None:
+    """A docstring mention is not removal of the signature parameter."""
+    docstring_only = "-        auto_init_config: Create .project.yaml if missing. Default: True\n"
+    changed = [_PRODUCT, _CONFTEST]
+
+    assert parity_harness_feature_paths(changed, {_CONFTEST: _KWARG_DIFF}, docstring_only) == [_CONFTEST]
+
+
+def test_unrelated_harness_lines_are_not_compatibility() -> None:
+    imported = _KWARG_DIFF + "+import os\n"
+    branched = _POP_DIFF + "+        if True:\n"
+    changed = [_PRODUCT, _CONFTEST, _OPENAI]
+    diffs = {_CONFTEST: imported, _OPENAI: branched}
+
+    assert parity_harness_feature_paths(changed, diffs, _SIGNATURE_DIFF) == [_CONFTEST, _OPENAI]

tests/darnit/parity/tier2/backends/openai_backend.py

@@ -80,8 +80,8 @@ def _dispatch_tool_call(call: Any, fixture_dir: Path) -> str:
         args["local_path"] = str(fixture_dir)
         args["output_format"] = "json"
         args["level"] = 3
-        # Force safe defaults.
-        args["auto_init_config"] = False
+        # Force safe defaults. Drop the removed no-op if a model still sends it.
+        args.pop("auto_init_config", None)
         args["attest"] = False
         args["prefer_upstream"] = False
 

tests/darnit/parity/tier2/run.py

@@ -74,7 +74,6 @@ def _run_mcp_tool(fixture_dir: Path) -> tuple[str, AuditResult]:
         local_path=str(fixture_dir),
         level=3,
         output_format="json",
-        auto_init_config=False,
         attest=False,
         prefer_upstream=False,
     )

tests/darnit/tools/test_framework_metadata.py

@@ -110,7 +110,6 @@ def test_json_output_has_top_level_metadata_block(self, tmp_path, monkeypatch):
             raw = audit_openssf_baseline(
                 local_path=str(tmp_path),
                 output_format="json",
-                auto_init_config=False,
                 attest=False,
                 prefer_upstream=False,
             )

tests/darnit_baseline/test_tools_signatures.py

@@ -93,3 +93,39 @@ def test_audit_handles_missing_path(self) -> None:
 
         result = audit_openssf_baseline(local_path="/nonexistent/path/12345")
         assert "Error" in result or "not found" in result.lower()
+
+
+class TestRemovedMcpParameters:
+    """Issue #528: ignored MCP parameters stay off the signatures and schemas."""
+
+    def test_signatures_omit_ignored_parameters(self) -> None:
+        import inspect
+
+        from darnit_baseline.tools import audit_openssf_baseline, get_pending_data
+
+        audit_params = inspect.signature(audit_openssf_baseline).parameters
+        pending_params = inspect.signature(get_pending_data).parameters
+
+        assert "auto_init_config" not in audit_params
+        assert "level" in audit_params
+        assert "level" not in pending_params
+        assert "profile" in pending_params
+
+    def test_mcp_schemas_omit_ignored_parameters(self) -> None:
+        import asyncio
+        from importlib.resources import files
+        from pathlib import Path
+
+        from darnit.server.factory import create_server
+
+        toml = Path(str(files("darnit_baseline") / "openssf-baseline.toml"))
+        server = create_server(toml)
+        tools = {tool.name: tool for tool in asyncio.run(server.list_tools())}
+
+        audit_props = tools["audit_openssf_baseline"].inputSchema["properties"]
+        pending_props = tools["get_pending_data"].inputSchema["properties"]
+
+        assert "auto_init_config" not in audit_props
+        assert "level" in audit_props
+        assert "level" not in pending_props
+        assert "profile" in pending_props

vulture_allowlist.py

@@ -3,8 +3,6 @@
 # These entries suppress vulture warnings for code that appears unused but is
 # intentionally kept. Vulture allowlists use attribute access to mark names used.
 
-# MCP tool parameters: part of the public MCP tool schema, exposed to AI
-# assistants even though the function body doesn't use them yet.
-# Removing them would change the tool's API contract.
-_ = auto_init_config  # noqa: F821  # audit_openssf_baseline parameter
+# MCP tool parameter kept on the public schema. Vulture allowlists use
+# attribute access to mark a name used.
 _ = attest  # noqa: F821  # audit_openssf_baseline parameter