Closes the four remaining findings from the margaret-hamilton review. 13 new regression tests; all 100 pass; live cluster smoke verified. MAJOR #4 — wildcard regex catastrophic backtracking + silent malformed. Two changes to _wildcard_to_regex(): a) Bounded the `!` and `@` wildcards to \d{1,50} (was \d+). Adjacent `!` patterns previously compiled to (\d+)(\d+)... which has exponential backtracking on near-miss inputs. CUCM dial strings are practically capped well below 50 digits; the bound keeps complexity polynomial without losing real-world coverage. Verified: 10 adjacent `!` against a 30-digit near-miss now finishes in ~240ms (was unbounded; could have been minutes on real pathological cases). b) Unclosed `[` now raises ValueError instead of silently treating the bracket as a literal. _pattern_matches_number catches the error and returns False so a single bad pattern doesn't crash translation_chain — but the bad pattern is no longer invisibly producing wrong matches. The previous silent fallback meant a pattern like `[0-9` (typo, missing `]`) would match input containing the literal characters `[` `0` `-` `9`. 3 new tests covering: bounded-regex shape (`\d{1,N}`), pathological input completes quickly, unclosed bracket raises explicitly, well-formed character class still works. MAJOR #5 — distinguish config errors from operational errors. Pre-fix: any first-time connection failure set `_connection_error` and pinned it forever. A transient network blip or session timeout required restarting the MCP server. Hamilton's framing: Apollo's software was *designed* to recover from transient faults; pinning forever is the antithesis of "design the error path first." Fix: split into two state fields: _config_error — permanent until restart (missing env vars only) _last_error — last operational failure, NOT a pin Operational failures (zeep Client construction, network, TLS, session) clear from the next call's perspective: the next call attempts fresh. Configuration errors (missing AXL_URL etc.) stay pinned because they don't get better on retry. Added _ConfigError as a private subclass to make the distinction explicit at the raise site, and connection_status() to expose connected/connected_at/config_error/last_error for diagnostic transparency. 3 new tests: config errors pin, operational errors don't pin, connection_status() reports state. MINOR #6 — _to_int silent coercion of bad data. Pre-fix: a non-numeric value from the cluster (data corruption, schema drift across CUCM versions) silently became None, which downstream sort logic defaulted to 0 — jumbling the failover order in the displayed result with no warning. Fix: still returns None on bad data (caller error path unchanged), but logs the offending value to stderr so an operator notices something's wrong at the data layer. None itself is silent (legitimately-unset column). 2 new tests: real None is silent, bad string logs to stderr with the offending value visible. MINOR #7 — standardize tool failure shapes; add health() tool. Pre-fix: cache_stats and cache_clear returned `{"error": "..."}` when _cache was None, while AXL-touching tools raised RuntimeError. LLM consumers had to handle two shapes. Fix: _require_cache() helper raises RuntimeError consistently with _client(). All tool failures now use the same exception shape. Added health() tool that reports cache/axl/docs initialization status plus the AXL connection_status — gives operators a self-diagnostic when something fails at bootstrap. 3 new tests: cache_stats raises, cache_clear raises, health() reports each subsystem.
155 lines
5.6 KiB
Python
155 lines
5.6 KiB
Python
"""Tests for CUCM dial-plan wildcard pattern matching."""
|
|
|
|
import pytest
|
|
|
|
from mcp_cucm_axl.route_plan import _pattern_matches_number, _wildcard_to_regex
|
|
|
|
|
|
class TestLiteralPatterns:
|
|
def test_exact_match(self):
|
|
assert _pattern_matches_number("1001", "1001")
|
|
|
|
def test_no_match(self):
|
|
assert not _pattern_matches_number("1001", "1002")
|
|
|
|
def test_escaped_plus(self):
|
|
assert _pattern_matches_number(r"\+15551234567", "+15551234567")
|
|
assert not _pattern_matches_number(r"\+15551234567", "15551234567")
|
|
|
|
|
|
class TestXWildcard:
|
|
def test_X_matches_any_digit(self):
|
|
assert _pattern_matches_number("XXXX", "1234")
|
|
assert _pattern_matches_number("XXXX", "9999")
|
|
|
|
def test_X_only_matches_digits(self):
|
|
assert not _pattern_matches_number("XXXX", "abc1")
|
|
assert not _pattern_matches_number("XXXX", "12") # too short
|
|
assert not _pattern_matches_number("XXXX", "12345") # too long
|
|
|
|
def test_X_mixed_with_literal(self):
|
|
assert _pattern_matches_number("9XXXX", "91234")
|
|
assert not _pattern_matches_number("9XXXX", "81234")
|
|
|
|
|
|
class TestBangWildcard:
|
|
def test_bang_matches_one_or_more(self):
|
|
assert _pattern_matches_number("9!", "91")
|
|
assert _pattern_matches_number("9!", "915551234567")
|
|
|
|
def test_bang_requires_at_least_one(self):
|
|
assert not _pattern_matches_number("9!", "9")
|
|
|
|
|
|
class TestCharacterClass:
|
|
def test_class_matches_any_in_set(self):
|
|
assert _pattern_matches_number("[2-9]XXX", "2000")
|
|
assert _pattern_matches_number("[2-9]XXX", "9999")
|
|
|
|
def test_class_excludes_outside_set(self):
|
|
assert not _pattern_matches_number("[2-9]XXX", "1000")
|
|
assert not _pattern_matches_number("[2-9]XXX", "0000")
|
|
|
|
|
|
class TestDotTerminator:
|
|
def test_dot_is_zero_width(self):
|
|
# CUCM's '.' is a marker for digit-discard, not a regex char
|
|
assert _pattern_matches_number("9.911", "9911")
|
|
assert _pattern_matches_number("10.911", "10911")
|
|
|
|
def test_dot_with_X_after(self):
|
|
assert _pattern_matches_number("9.[2-9]XXXXXXXXX", "92085551234")
|
|
|
|
|
|
class TestAtPattern:
|
|
def test_at_matches_any_digits(self):
|
|
# @ would normally apply a route filter; we treat it as "any digits"
|
|
assert _pattern_matches_number("9.@", "915551234567")
|
|
assert _pattern_matches_number("\\+.@", "+15551234567")
|
|
|
|
|
|
class TestEdgeCases:
|
|
def test_invalid_regex_returns_false(self):
|
|
# An unbalanced bracket should not raise
|
|
result = _pattern_matches_number("[", "1")
|
|
assert result is False
|
|
|
|
def test_empty_pattern(self):
|
|
assert _pattern_matches_number("", "")
|
|
assert not _pattern_matches_number("", "1")
|
|
|
|
def test_regex_anchors(self):
|
|
# Make sure we don't match a substring
|
|
assert not _pattern_matches_number("911", "1911")
|
|
assert not _pattern_matches_number("911", "9111")
|
|
|
|
|
|
class TestRegexConversion:
|
|
def test_X_to_digit_class(self):
|
|
# Bounded after Hamilton MAJOR #4 — `X` still matches a single digit
|
|
assert _wildcard_to_regex("X") == r"^\d$"
|
|
|
|
def test_bang_to_bounded_digits(self):
|
|
# Bounded after Hamilton MAJOR #4 — was \d+, now \d{1,N}.
|
|
# Adjacent !!! used to compile to (\d+)(\d+)(\d+) which has
|
|
# exponential backtracking on near-miss inputs.
|
|
regex = _wildcard_to_regex("!")
|
|
# Must be anchored, must contain a digit class with an upper bound.
|
|
assert regex.startswith("^") and regex.endswith("$")
|
|
assert r"\d{1," in regex, (
|
|
f"`!` must compile to bounded `\\d{{1,N}}` to prevent "
|
|
f"catastrophic backtracking; got: {regex}"
|
|
)
|
|
|
|
def test_anchored(self):
|
|
regex = _wildcard_to_regex("9XXX")
|
|
assert regex.startswith("^")
|
|
assert regex.endswith("$")
|
|
|
|
|
|
class TestUnclosedBracketIsExplicitError:
|
|
"""Hamilton review MAJOR #4 (part 2): unclosed `[` used to silently
|
|
fall back to treating the bracket as a literal. That produced wrong
|
|
matches with no warning. Fix: surface the malformed pattern as an
|
|
explicit error so the caller can flag it.
|
|
"""
|
|
|
|
def test_unclosed_bracket_raises(self):
|
|
import pytest as _pytest
|
|
with _pytest.raises(ValueError, match="bracket"):
|
|
_wildcard_to_regex("[0-9")
|
|
|
|
def test_unclosed_bracket_in_pattern_match_returns_false(self):
|
|
# _pattern_matches_number must catch the ValueError from the
|
|
# malformed pattern and return False (so a single bad pattern
|
|
# doesn't crash translation_chain).
|
|
assert _pattern_matches_number("[0-9", "1") is False
|
|
|
|
def test_well_formed_bracket_still_works(self):
|
|
# Sanity: the fix shouldn't break legitimate character classes
|
|
assert _pattern_matches_number("[2-9]XX", "456")
|
|
assert not _pattern_matches_number("[2-9]XX", "156")
|
|
|
|
|
|
class TestRegexBacktrackingBound:
|
|
"""Hamilton review MAJOR #4 (part 1): adjacent `!` wildcards used to
|
|
compile to `\\d+\\d+\\d+...` which is exponentially slow on near-miss
|
|
input. Bounded `\\d{1,N}` keeps it polynomial.
|
|
"""
|
|
|
|
def test_pathological_pattern_completes_quickly(self):
|
|
# 10 adjacent `!` matched against a long near-miss number.
|
|
# Pre-fix this could take seconds; bounded should finish in ms.
|
|
import time
|
|
pat = "!" * 10
|
|
# 30 digits + a trailing letter — guarantees no full match
|
|
num = "1" * 30 + "X"
|
|
t0 = time.monotonic()
|
|
result = _pattern_matches_number(pat, num)
|
|
elapsed = time.monotonic() - t0
|
|
assert result is False
|
|
assert elapsed < 0.5, (
|
|
f"pathological `!` chain must finish quickly even on near-miss; "
|
|
f"took {elapsed:.3f}s"
|
|
)
|