You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/09/07 23:09:48 UTC

(magpie) branch main updated: fix(validator): tokenize gh-list-no-limit's flag check instead of substring match (#1147)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git


The following commit(s) were added to refs/heads/main by this push:
     new 7dfb956e fix(validator): tokenize gh-list-no-limit's flag check instead of substring match (#1147)
7dfb956e is described below

commit 7dfb956e45c9900433994a7e7550f2b9062d33be
Author: Amir Fathi <am...@gmail.com>
AuthorDate: Mon Sep 7 17:09:42 2026 -0600

    fix(validator): tokenize gh-list-no-limit's flag check instead of substring match (#1147)
    
    validate_gh_list_limit tested for the literal substring "--limit" anywhere on
    the logical line, so a trailing shell comment or a quoted argument that
    merely mentions "--limit" silently defeated the check even when the command
    had no such flag. Tokenize with shlex.split(comments=True) and match an
    actual --limit / --limit=N token instead.
    
    Fixes #1146
    
    Generated-by: Claude Code (Sonnet 5)
---
 .../src/skill_and_tool_validator/__init__.py            |  7 ++++++-
 tools/skill-and-tool-validator/tests/test_validator.py  | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py
index 8e720802..acae007c 100644
--- a/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py
+++ b/tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py
@@ -151,6 +151,7 @@ from __future__ import annotations
 import argparse
 import contextlib
 import re
+import shlex
 import subprocess
 import sys
 from collections.abc import Iterable
@@ -2726,7 +2727,11 @@ def validate_gh_list_limit(path: Path, text: str) -> Iterable[Violation]:
             if line_end == -1:
                 line_end = len(joined)
             logical_line = joined[line_start:line_end]
-            if "--limit" in logical_line:
+            try:
+                tokens = shlex.split(logical_line, comments=True)
+            except ValueError:
+                tokens = []  # unbalanced quoting, can't tell, so flag it
+            if any(tok == "--limit" or tok.startswith("--limit=") for tok in tokens):
                 continue
             line_no = text[: block_match.start()].count("\n") + joined[: cmd_match.start()].count("\n") + 1
             yield Violation(
diff --git a/tools/skill-and-tool-validator/tests/test_validator.py b/tools/skill-and-tool-validator/tests/test_validator.py
index 361faf15..b729a4cb 100644
--- a/tools/skill-and-tool-validator/tests/test_validator.py
+++ b/tools/skill-and-tool-validator/tests/test_validator.py
@@ -1800,6 +1800,23 @@ class TestGhListLimit:
         violations = list(validate_gh_list_limit(path, text))
         assert not any("gh-list-no-limit" in v.message for v in violations)
 
+    def test_fires_when_limit_only_appears_in_a_trailing_comment(self, tmp_path: Path) -> None:
+        path = tmp_path / "SKILL.md"
+        text = _fenced("gh issue list --repo <repo> --state open  # add --limit later if slow")
+        violations = list(validate_gh_list_limit(path, text))
+        assert any("gh-list-no-limit" in v.message for v in violations)
+
+    def test_fires_when_limit_only_appears_inside_a_quoted_argument(self, tmp_path: Path) -> None:
+        path = tmp_path / "SKILL.md"
+        text = _fenced('gh issue list --repo <repo> --search "mentions --limit"')
+        violations = list(validate_gh_list_limit(path, text))
+        assert any("gh-list-no-limit" in v.message for v in violations)
+
+    def test_silent_when_limit_uses_equals_form(self, tmp_path: Path) -> None:
+        path = tmp_path / "SKILL.md"
+        violations = list(validate_gh_list_limit(path, _fenced("gh issue list --repo <repo> --limit=100")))
+        assert not any("gh-list-no-limit" in v.message for v in violations)
+
 
 # ---------------------------------------------------------------------------
 # Pattern 6 — Privacy-LLM gate-check