mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(man): accept canonical name(section) page notation
Models often emit page references in the standard man-page form (``printf(3)``, ``open(2)``, ``perlfunc(3pm)``) rather than splitting them into ``page`` + ``section`` args. The page-name sanitizer was rejecting the parens as invalid input, killing the call. Parse the section out of the page string before sanitization (explicit ``section`` arg still wins) and widen the section validator to accept multi-letter suffixes like ``3pm`` / ``3perl`` that already appear on real systems.
This commit is contained in:
@@ -531,6 +531,81 @@ class TestAgentModelOverride:
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# man tool
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPrepareMan:
|
||||
"""``ChatSession._prepare_man`` argument parsing."""
|
||||
|
||||
def test_plain_page(self, tmp_db) -> None:
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "grep"})
|
||||
assert "error" not in item
|
||||
assert item["page"] == "grep"
|
||||
assert item["section"] == ""
|
||||
|
||||
def test_explicit_section_arg(self, tmp_db) -> None:
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "printf", "section": "3"})
|
||||
assert "error" not in item
|
||||
assert item["page"] == "printf"
|
||||
assert item["section"] == "3"
|
||||
|
||||
def test_parenthesized_section_in_page(self, tmp_db) -> None:
|
||||
# Models commonly emit canonical man-page notation; we should
|
||||
# parse the section out instead of rejecting the call.
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "printf(3)"})
|
||||
assert "error" not in item
|
||||
assert item["page"] == "printf"
|
||||
assert item["section"] == "3"
|
||||
assert "printf(3)" in item["header"]
|
||||
|
||||
def test_parenthesized_section_with_letter_suffix(self, tmp_db) -> None:
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "perlfunc(3pm)"})
|
||||
assert "error" not in item
|
||||
assert item["page"] == "perlfunc"
|
||||
assert item["section"] == "3pm"
|
||||
|
||||
def test_explicit_section_arg_wins_over_parsed(self, tmp_db) -> None:
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "open(2)", "section": "3"})
|
||||
assert "error" not in item
|
||||
assert item["page"] == "open"
|
||||
assert item["section"] == "3"
|
||||
|
||||
def test_invalid_section_in_parens_falls_through_to_error(self, tmp_db) -> None:
|
||||
# Parens that don't match the section pattern aren't parsed away,
|
||||
# so the page-name sanitizer rejects the literal string.
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "grep(bogus)"})
|
||||
assert "error" in item
|
||||
assert "invalid page name" in item["error"]
|
||||
|
||||
def test_empty_page(self, tmp_db) -> None:
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": ""})
|
||||
assert "error" in item
|
||||
assert "no page name" in item["error"]
|
||||
|
||||
def test_parsed_section_reaches_subprocess_argv(self, tmp_db) -> None:
|
||||
# End-to-end check that page="printf(3)" produces the right
|
||||
# ``man`` argv — guards against future drift between
|
||||
# ``_prepare_man``'s output keys and ``_exec_man``'s reads.
|
||||
session = _make_session()
|
||||
item = session._prepare_man("c1", {"page": "printf(3)"})
|
||||
completed = subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout="MAN PAGE TEXT", stderr=""
|
||||
)
|
||||
with patch("subprocess.run", return_value=completed) as mock_run:
|
||||
session._exec_man(item)
|
||||
argv = mock_run.call_args_list[0].args[0]
|
||||
assert argv == ["man", "3", "printf"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Plan validation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -5237,6 +5237,16 @@ class ChatSession:
|
||||
"needs_approval": False,
|
||||
"error": "Error: no page name provided",
|
||||
}
|
||||
section = (args.get("section") or "").strip()
|
||||
# Accept the canonical "name(section)" notation that the model often
|
||||
# emits (e.g. printf(3), open(2), perlfunc(3pm)) \u2014 the parens are
|
||||
# otherwise rejected by the page-name sanitizer below. An explicit
|
||||
# ``section`` arg, if provided, takes precedence over the parsed one.
|
||||
m = re.match(r"^([a-zA-Z0-9._-]+)\(([1-9][a-z]*)\)$", page)
|
||||
if m:
|
||||
page = m.group(1)
|
||||
if not section:
|
||||
section = m.group(2)
|
||||
# Sanitize: only allow alphanumeric, dash, underscore, dot
|
||||
if not re.match(r"^[a-zA-Z0-9._-]+$", page):
|
||||
return {
|
||||
@@ -5247,8 +5257,7 @@ class ChatSession:
|
||||
"needs_approval": False,
|
||||
"error": f"Error: invalid page name {page!r}",
|
||||
}
|
||||
section = (args.get("section") or "").strip()
|
||||
if section and not re.match(r"^[1-9][a-z]?$", section):
|
||||
if section and not re.match(r"^[1-9][a-z]*$", section):
|
||||
section = ""
|
||||
label = f"{page}({section})" if section else page
|
||||
preview = f" {DIM}{label}{RESET}"
|
||||
|
||||
Reference in New Issue
Block a user