diff --git a/engine/hooks/pr-schema-gate/README.md b/engine/hooks/pr-schema-gate/README.md index 958caa21..917778ac 100644 --- a/engine/hooks/pr-schema-gate/README.md +++ b/engine/hooks/pr-schema-gate/README.md @@ -82,7 +82,9 @@ push` and moved on without running `create-pr.mjs --update-existing`. - `shell_model.py`: boundary parser from a tool call (Claude/Cursor `command`, Codex `cmd`, argv lists, and Codex's JavaScript-wrapped `exec_command({...})`) to `Command(argv, cwd)` values, following `cd` - and explicit `workdir`. + and explicit `workdir`. A quoted argument that spans lines (a multi-line + `git commit -m "..."`) stays one word; only a quote that never closes + makes the command unparseable. - `detect.py`: classification of commands, target-repo resolution, the validator call, and the pending state. - `claude_pretooluse.py`: the `PreToolUse` entrypoint for all three diff --git a/engine/hooks/pr-schema-gate/shell_model.py b/engine/hooks/pr-schema-gate/shell_model.py index 50487ee7..a40b9644 100644 --- a/engine/hooks/pr-schema-gate/shell_model.py +++ b/engine/hooks/pr-schema-gate/shell_model.py @@ -21,6 +21,7 @@ SEPARATORS = frozenset({";", "&&", "||", "|", "&", "|&", "(", ")", ";;"}) HEREDOC_OPERATORS = frozenset({"<<", "<<-"}) +INCOMPLETE_INPUT_ERRORS = frozenset({"No closing quotation", "No escaped character"}) @dataclass(frozen=True) @@ -99,6 +100,16 @@ def _tokenize(line: str) -> list[str]: return list(lexer) +def _tokenize_until_quote_closes(text: str) -> list[str] | None: + """Tokens of `text`, or None while the input is incomplete. Any other lexer error propagates.""" + try: + return _tokenize(text) + except ValueError as exc: + if str(exc) in INCOMPLETE_INPUT_ERRORS: + return None + raise + + def _logical_lines(script: str) -> list[str]: lines: list[str] = [] pending = "" @@ -127,9 +138,14 @@ def _heredoc_delimiters(tokens: list[str]) -> list[tuple[str, bool]]: def _token_lines(script: str) -> list[list[str]] | None: - """Tokenize each logical line, dropping heredoc bodies. None if any line cannot be lexed.""" + """Tokenize each logical line, dropping heredoc bodies. None if a quote never closes. + + A line that cannot be lexed alone opens a quote; the following lines join it + until the quote closes, so a quoted argument spanning lines stays one word. + """ result: list[list[str]] = [] waiting: list[tuple[str, bool]] = [] + open_quote: str | None = None for line in _logical_lines(script): if waiting: delimiter, strip_tabs = waiting[0] @@ -137,13 +153,15 @@ def _token_lines(script: str) -> list[list[str]] | None: if candidate.strip() == delimiter: waiting.pop(0) continue - try: - tokens = _tokenize(line) - except ValueError: - return None - waiting.extend(_heredoc_delimiters(tokens)) - result.append(tokens) - return result + text = line if open_quote is None else open_quote + "\n" + line + tokens = _tokenize_until_quote_closes(text) + if tokens is None: + open_quote = text + else: + open_quote = None + waiting.extend(_heredoc_delimiters(tokens)) + result.append(tokens) + return None if open_quote is not None else result def _is_assignment(word: str) -> bool: diff --git a/engine/hooks/pr-schema-gate/tests/test_multiline_quotes.py b/engine/hooks/pr-schema-gate/tests/test_multiline_quotes.py new file mode 100644 index 00000000..44bd7ac5 --- /dev/null +++ b/engine/hooks/pr-schema-gate/tests/test_multiline_quotes.py @@ -0,0 +1,81 @@ +"""A quoted argument that spans lines is one word, not an unparseable command. + +The payload is the real command that drew a false "could not parse" advisory: +a heredoc, then a `git commit -am "\n\n"` whose message spans +three lines. +""" +from __future__ import annotations + +import io +import json +import os +import sys +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) +import claude_pretooluse # noqa: E402 +import detect # noqa: E402 +from shell_model import ShellCall, parse_commands # noqa: E402 + +MESSAGE = ( + "principle-explicit-errors: drop an unobserved count from the example\n\n" + "Co-Authored-By: Claude Opus 5 (1M context) \n" + "Claude-Session: https://claude.ai/code/session_01WSVNdBBo7dFqoR8Tx521EJ" +) +REAL_COMMAND = ( + "cd /work && python3 - <<'EOF'\n" + "p = \"tests/fires_example.md\"\n" + "open(p, \"w\").write(\"x\")\n" + "EOF\n" + "grep -n \"rewords\" tests/fires_example.md && git commit -q -am \"" + MESSAGE + "\" && git log --oneline -2" +) + + +class TestMultilineQuotedArguments(unittest.TestCase): + def test_real_multiline_commit_message_parses(self): + commands = parse_commands(ShellCall(REAL_COMMAND, None), "/s") + self.assertIsNotNone(commands) + argvs = [c.argv for c in commands] + self.assertIn(("git", "commit", "-q", "-am", MESSAGE), argvs) + self.assertIn(("git", "log", "--oneline", "-2"), argvs) + self.assertIn(("grep", "-n", "rewords", "tests/fires_example.md"), argvs) + + def test_single_quoted_multiline_argument_parses(self): + commands = parse_commands(ShellCall("echo 'one\ntwo'\necho three", None), "/s") + self.assertEqual([c.argv for c in commands], [("echo", "one\ntwo"), ("echo", "three")]) + + def test_quote_never_closed_is_still_unparseable(self): + self.assertIsNone(parse_commands(ShellCall("echo 'one\ntwo\nthree", None), "/s")) + + def test_unclosed_quote_ending_in_a_backslash_is_unparseable_not_an_error(self): + self.assertIsNone(parse_commands(ShellCall('echo "a\\', None), "/s")) + + def test_trailing_backslash_outside_quotes_is_a_line_continuation(self): + commands = parse_commands(ShellCall("echo done \\", None), "/s") + self.assertEqual([c.argv for c in commands], [("echo", "done")]) + + def test_hook_is_silent_on_the_real_command_in_scope(self): + with tempfile.TemporaryDirectory() as repo: + os.makedirs(os.path.join(repo, "scripts")) + os.makedirs(os.path.join(repo, ".git")) + open(os.path.join(repo, "scripts", "create-pr.mjs"), "w").close() + state = tempfile.TemporaryDirectory() + self.addCleanup(state.cleanup) + os.environ[detect.STATE_DIR_ENV] = state.name + self.addCleanup(os.environ.pop, detect.STATE_DIR_ENV, None) + payload = {"tool_name": "Bash", "tool_input": {"command": REAL_COMMAND}, "cwd": repo} + err, out = io.StringIO(), io.StringIO() + old = sys.stdin + sys.stdin = io.StringIO(json.dumps(payload)) + try: + with redirect_stderr(err), redirect_stdout(out): + claude_pretooluse.main() + finally: + sys.stdin = old + self.assertEqual((err.getvalue(), out.getvalue()), ("", "")) + + +if __name__ == "__main__": + unittest.main()