fix(integrations): reject empty --commands-dir in generic raw_options (#3714)

* fix(integrations): reject empty --commands-dir in generic raw_options

GenericIntegration._resolve_commands_dir has a parity gap: the parsed-options
branch guards emptiness (`if commands_dir:`), but the raw_options fallback
returned the value verbatim with no check. So `--integration-options=
"--commands-dir="` (or `--commands-dir ""`) resolves to `""`, which makes
setup() compute `dest = project_root / "" == project_root` and write every
speckit command file (specify.md, plan.md, ...) directly into the PROJECT
ROOT — silently bypassing the documented "--commands-dir is required"
contract and polluting the repo root.

Apply the same non-empty guard to the raw_options branch so an empty value
falls through to the existing "required" ValueError on every input form.
Non-empty values resolve exactly as before.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(integrations): reject a BLANK --commands-dir, not just an empty one

Self-review follow-up: bare truthiness only closes the empty-string subset. A
whitespace-only value passed both branches (verified: raw "--commands-dir ' '"
returned ' ', parsed {"commands_dir": "  "} returned '  '), so command files
still landed in a directory literally named " " instead of failing with the
documented "required" error.

Require a non-BLANK value and normalize the padding, in the parsed branch as
well as raw_options so the two cannot drift apart -- a padded but real value
(" .myagent/cmds ") now resolves to ".myagent/cmds" rather than being rejected,
matching how other padded config references are normalized.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(integrations): use strip() only to test blankness, return the value verbatim

Address review feedback: normalizing with strip() changed EXISTING valid values,
contrary to this PR's "no behaviour change for valid usage" claim -- a quoted
`--commands-dir ' commands '` previously targeted the literal ` commands `
directory and would have started writing to `commands` instead.

The blankness test still uses strip(), but the accepted value is now returned
unchanged, so the fix stays limited to empty/blank input. Test updated
accordingly: a padded non-blank value must round-trip verbatim (quoted in
raw_options, since shlex.split() consumes unquoted padding before this code
sees it).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/specify_cli/integrations/generic/__init__.py

@@ -53,8 +53,16 @@ class GenericIntegration(MarkdownIntegration):
         """
         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 @@ class GenericIntegration(MarkdownIntegration):
             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"

tests/integrations/test_integration_generic.py

@@ -55,6 +55,62 @@ class TestGenericIntegration:
         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)