mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(relay): normalize a 0/negative max_message_length at the descriptor boundary
Follow-up to the truncate_message split-loop floor. Two review points:
- CapabilityDescriptor.from_json trusted the wire max_message_length
verbatim, so a connector advertising 0 ('no limit') — or a buggy one
sending 0/negative — produced a descriptor whose bound flowed straight
into the adapter's MAX_MESSAGE_LENGTH and truncate_message. Normalize
it to the documented 4096 default (mirrors from_platform_entry's
'or 4096' and docs/relay-connector-contract.md), fixing the degenerate
budget at its source rather than only surviving it downstream.
- Document the truncate_message length contract for a budget too small
for one codepoint (max_length=1 with a 2-unit surrogate pair under
utf16_len): the chunk intentionally exceeds max_length by that one
indivisible codepoint, because emitting it whole preserves content
where the alternatives are data loss or an infinite loop.
Tests: from_json normalizes 0 and negative bounds to 4096 and passes a
real positive bound through unchanged; the sub-codepoint budget emits
whole codepoints with no data loss (all emojis preserved) and a chunk
that necessarily exceeds the 1-unit budget.
This commit is contained in:
parent
fbf5005a7e
commit
27b31bb7ff
4 changed files with 78 additions and 0 deletions
|
|
@ -5694,6 +5694,17 @@ class BasePlatformAdapter(ABC):
|
|||
# the whole budget — leaves split_at at 0, so ``remaining``
|
||||
# never shrinks and the while-loop spins forever appending
|
||||
# empty chunks (an unbounded hang / OOM).
|
||||
#
|
||||
# Length contract for a degenerate budget: a codepoint is the
|
||||
# smallest indivisible unit, so when the budget is smaller than
|
||||
# one codepoint (e.g. max_length=1 with a 2-unit surrogate pair
|
||||
# under utf16_len) the emitted chunk WILL exceed max_length by
|
||||
# that one codepoint. That is intentional — emitting the
|
||||
# codepoint whole preserves the content, whereas the only
|
||||
# alternatives are dropping it (data loss) or looping forever.
|
||||
# Real callers never hit this: platform caps are hundreds/
|
||||
# thousands, and the relay path normalizes a 0/negative
|
||||
# descriptor bound to 4096 (see gateway/relay/descriptor.py).
|
||||
split_at = max(1, _cp_limit)
|
||||
|
||||
# Avoid splitting inside an inline code span (`...`).
|
||||
|
|
|
|||
|
|
@ -80,6 +80,19 @@ class CapabilityDescriptor:
|
|||
raw = json.loads(data)
|
||||
known = {f for f in cls.__dataclass_fields__} # type: ignore[attr-defined]
|
||||
filtered = {k: v for k, v in raw.items() if k in known}
|
||||
# Normalize the chunking bound at the trust boundary. A connector may
|
||||
# advertise max_message_length 0 ("no limit"), and a buggy/hostile one
|
||||
# may send 0 or a negative; either is a degenerate value that would flow
|
||||
# straight into the adapter's MAX_MESSAGE_LENGTH and truncate_message().
|
||||
# Map it to the documented 4096 default (docs/relay-connector-contract.md;
|
||||
# mirrors from_platform_entry's `or 4096`) so from_json never yields a
|
||||
# descriptor that can't chunk a real message.
|
||||
if "max_message_length" in filtered:
|
||||
try:
|
||||
if int(filtered["max_message_length"]) <= 0:
|
||||
filtered["max_message_length"] = 4096
|
||||
except (TypeError, ValueError):
|
||||
filtered["max_message_length"] = 4096
|
||||
return cls(**filtered)
|
||||
|
||||
@classmethod
|
||||
|
|
|
|||
|
|
@ -46,6 +46,38 @@ def test_from_json_ignores_unknown_keys():
|
|||
assert restored == d
|
||||
|
||||
|
||||
def test_from_json_normalizes_zero_max_message_length_to_default():
|
||||
"""A connector may advertise max_message_length 0 ("no limit"). from_json
|
||||
must normalize it to the documented 4096 default so the receiver never
|
||||
carries a degenerate chunking bound into truncate_message()."""
|
||||
raw = (
|
||||
'{"contract_version": 1, "platform": "x", "label": "X", '
|
||||
'"max_message_length": 0, "supports_draft_streaming": false, '
|
||||
'"supports_edit": false, "supports_threads": false, '
|
||||
'"markdown_dialect": "plain", "len_unit": "chars"}'
|
||||
)
|
||||
d = CapabilityDescriptor.from_json(raw)
|
||||
assert d.max_message_length == 4096
|
||||
|
||||
|
||||
def test_from_json_normalizes_negative_max_message_length_to_default():
|
||||
"""A buggy/hostile connector sending a negative bound is normalized too."""
|
||||
raw = (
|
||||
'{"contract_version": 1, "platform": "x", "label": "X", '
|
||||
'"max_message_length": -5, "supports_draft_streaming": false, '
|
||||
'"supports_edit": false, "supports_threads": false, '
|
||||
'"markdown_dialect": "plain", "len_unit": "chars"}'
|
||||
)
|
||||
d = CapabilityDescriptor.from_json(raw)
|
||||
assert d.max_message_length == 4096
|
||||
|
||||
|
||||
def test_from_json_keeps_a_real_positive_bound():
|
||||
"""A normal positive bound is passed through unchanged."""
|
||||
d = CapabilityDescriptor.from_json(_telegram_descriptor(max_message_length=2000).to_json())
|
||||
assert d.max_message_length == 2000
|
||||
|
||||
|
||||
def test_from_json_fills_optional_defaults():
|
||||
"""Optional fields (emoji/platform_hint/pii_safe) fall back to defaults."""
|
||||
minimal = (
|
||||
|
|
|
|||
|
|
@ -1651,6 +1651,28 @@ class TestTruncateMessage:
|
|||
assert chunks
|
||||
assert "😀" in "".join(chunks)
|
||||
|
||||
def test_sub_codepoint_budget_emits_whole_codepoints_without_data_loss(self):
|
||||
"""Length contract for a budget too small to fit one codepoint.
|
||||
|
||||
A codepoint is indivisible, so with max_length=1 and utf16_len a 2-unit
|
||||
emoji cannot fit — the loop emits it whole rather than dropping it or
|
||||
spinning. The documented, intentional consequence is that such a chunk
|
||||
EXCEEDS max_length by that one codepoint; in return every codepoint is
|
||||
preserved (no data loss) and the call terminates.
|
||||
"""
|
||||
import re
|
||||
|
||||
from gateway.platforms.base import utf16_len
|
||||
|
||||
chunks = self._truncate_with_timeout("😀😀😀", 1, len_fn=utf16_len)
|
||||
assert chunks
|
||||
bodies = [re.sub(r"\s*\(\d+/\d+\)$", "", c) for c in chunks]
|
||||
# No data loss: all three emojis survive across the chunks.
|
||||
assert "".join(bodies).count("😀") == 3
|
||||
# Contract: a chunk carrying a 2-unit emoji necessarily exceeds the
|
||||
# 1-unit budget — assert that explicitly so the behavior is pinned.
|
||||
assert any(utf16_len(b) > 1 for b in bodies)
|
||||
|
||||
def test_code_block_first_chunk_closed(self):
|
||||
adapter = self._adapter()
|
||||
msg = "Before\n```python\n" + "x = 1\n" * 100 + "```\nAfter"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue