diff --git a/src/specify_cli/integrations/generic/__init__.py b/src/specify_cli/integrations/generic/__init__.py index a2fd430f75..f8ea47ccb2 100644 --- a/src/specify_cli/integrations/generic/__init__.py +++ b/src/specify_cli/integrations/generic/__init__.py @@ -53,8 +53,16 @@ def _resolve_commands_dir( """ parsed_options = parsed_options or {} + # Accept a value only when it is non-BLANK. An empty value resolves to + # the project root (``project_root / ""``) and a whitespace-only one to + # a directory literally named " ", so either would silently scatter + # command files instead of failing with the documented "required" + # error. ``strip()`` is used ONLY to decide blankness -- the value + # itself is returned verbatim, so a deliberate (if unusual) padded + # directory name still targets exactly what the user asked for. Both + # branches below apply the same rule so they cannot drift apart. commands_dir = parsed_options.get("commands_dir") - if commands_dir: + if commands_dir and (not isinstance(commands_dir, str) or commands_dir.strip()): return commands_dir # Fall back to raw_options (--integration-options="--commands-dir ...") @@ -64,9 +72,13 @@ def _resolve_commands_dir( tokens = shlex.split(raw) for i, token in enumerate(tokens): if token == "--commands-dir" and i + 1 < len(tokens): - return tokens[i + 1] + candidate = tokens[i + 1] + if candidate.strip(): + return candidate if token.startswith("--commands-dir="): - return token.split("=", 1)[1] + candidate = token.split("=", 1)[1] + if candidate.strip(): + return candidate raise ValueError( "--commands-dir is required for the generic integration" diff --git a/tests/integrations/test_integration_generic.py b/tests/integrations/test_integration_generic.py index 3f079c438d..202f7ab3dd 100644 --- a/tests/integrations/test_integration_generic.py +++ b/tests/integrations/test_integration_generic.py @@ -55,6 +55,62 @@ def test_setup_requires_nonempty_commands_dir(self, tmp_path): with pytest.raises(ValueError, match="--commands-dir is required"): i.setup(tmp_path, m, parsed_options={"commands_dir": ""}) + @pytest.mark.parametrize("blank", [" ", "\t"]) + def test_resolve_commands_dir_rejects_blank_parsed_value(self, blank): + """A whitespace-only value must raise too: it resolves to a directory + literally named " ", scattering command files just like the empty case.""" + from specify_cli.integrations.generic import GenericIntegration + + with pytest.raises(ValueError, match="--commands-dir is required"): + GenericIntegration._resolve_commands_dir({"commands_dir": blank}, {}) + + @pytest.mark.parametrize( + "raw", ["--commands-dir ' '", "--commands-dir=' '", "--commands-dir '\t'"] + ) + def test_resolve_commands_dir_rejects_blank_raw_value(self, raw): + """Same rule on the raw_options branch, so the two cannot drift apart.""" + from specify_cli.integrations.generic import GenericIntegration + + with pytest.raises(ValueError, match="--commands-dir is required"): + GenericIntegration._resolve_commands_dir({}, {"raw_options": raw}) + + @pytest.mark.parametrize("padded", [" .myagent/cmds ", "\t.myagent/cmds"]) + def test_resolve_commands_dir_returns_padded_value_verbatim(self, padded): + """A padded but non-blank value is accepted and returned UNCHANGED: the + blankness test uses strip(), but rewriting the value would silently + retarget a directory the user asked for by name.""" + from specify_cli.integrations.generic import GenericIntegration + + assert GenericIntegration._resolve_commands_dir( + {"commands_dir": padded}, {} + ) == padded + # Quoted in raw_options, since shlex.split() would otherwise consume the + # surrounding whitespace before this code ever sees it. + assert GenericIntegration._resolve_commands_dir( + {}, {"raw_options": f"--commands-dir='{padded}'"} + ) == padded + + @pytest.mark.parametrize("raw", ["--commands-dir=", "--commands-dir ''", '--commands-dir ""']) + def test_resolve_commands_dir_rejects_empty_raw_value(self, raw): + """An empty --commands-dir in raw_options must raise the same "required" + error as the parsed-options path — not return "" (which resolves to the + project root and writes command files there). Mirrors the parsed branch.""" + from specify_cli.integrations.generic import GenericIntegration + + with pytest.raises(ValueError, match="--commands-dir is required"): + GenericIntegration._resolve_commands_dir({}, {"raw_options": raw}) + + def test_resolve_commands_dir_accepts_nonempty_raw_value(self): + """A non-empty raw --commands-dir still resolves unchanged.""" + from specify_cli.integrations.generic import GenericIntegration + + assert GenericIntegration._resolve_commands_dir( + {}, {"raw_options": "--commands-dir .myagent/commands"} + ) == ".myagent/commands" + assert GenericIntegration._resolve_commands_dir( + {}, {"raw_options": "--commands-dir=.myagent/commands"} + ) == ".myagent/commands" + def test_setup_writes_to_correct_directory(self, tmp_path): i = get_integration("generic") m = IntegrationManifest("generic", tmp_path)