From 9e67f4621d436402c4ab4e80627362544a1ca1df Mon Sep 17 00:00:00 2001 From: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:51:04 +0800 Subject: [PATCH 1/2] fix(lark): state the upload acknowledgement shape once `_uploaded_resources` built `re.fullmatch(key_prefix + tail, ...)` at both of its use sites, so the identifier-shape guard could not fold the value and reported the file as an unfoldable site: `return_files.py:75` when reading a saved acknowledgement back and `:100` when accepting a fresh one. The prefix was never caller data -- the two callers in this same file pass `file_` and `img_` as literals -- so both shapes are now stated once as module patterns and the use sites answer from them. The guard's other exit (declaring `return_files.py: 2`) was rejected: the two current declarations are justified because those sites decide no identifier shape at all, while this one does. Refs #7387 Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com> --- loopx/extensions/lark/return_files.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/loopx/extensions/lark/return_files.py b/loopx/extensions/lark/return_files.py index bf66b0714c..e503900a57 100644 --- a/loopx/extensions/lark/return_files.py +++ b/loopx/extensions/lark/return_files.py @@ -19,6 +19,13 @@ from ...file_lock import exclusive_file_lock +# An upload acknowledgement is the resource prefix plus 1-240 token characters. +# The prefix is a property of the resource kind, not of the caller, so both +# shapes are stated once here instead of being rebuilt at each use site. +RESULT_FILE_KEY_PATTERN = re.compile(r"file_[A-Za-z0-9_-]{1,240}") +CHAT_IMAGE_KEY_PATTERN = re.compile(r"img_[A-Za-z0-9_-]{1,240}") + + def _call(args, cwd, *, runner, private_transport): try: if private_transport is not None: @@ -36,7 +43,8 @@ def uploaded_result_files(*, root, profile, provider_ref, attachments, runner=No never uploads replacement resources to reconstruct a sent-message intent. """ return _uploaded_resources(root=root, profile=profile, provider_ref=provider_ref, attachments=attachments, - cache_dir="lark-result-files", resource="files", key_field="file_key", key_prefix="file_", + cache_dir="lark-result-files", resource="files", key_field="file_key", + key_pattern=RESULT_FILE_KEY_PATTERN, read_bytes=lambda attachment: read_result_file(root, attachment), upload_data=lambda attachment: {"file_type": "stream", "file_name": attachment["name"]}, runner=runner, private_transport=private_transport, before_upload=before_upload, verify_only=verify_only) @@ -55,14 +63,15 @@ def uploaded_chat_images(*, root, profile, provider_ref, attachments, private_tr """Reuse resource IO; never regenerate an unknown upload or sent intent.""" return _uploaded_resources(root=root, profile=profile, provider_ref=provider_ref, attachments=_chat_images(attachments), cache_dir="lark-chat-images", resource="images", - key_field="image_key", key_prefix="img_", read_bytes=lambda image: image["raw"], + key_field="image_key", key_pattern=CHAT_IMAGE_KEY_PATTERN, + read_bytes=lambda image: image["raw"], upload_data=lambda _image: {"image_type": "message"}, runner=None, private_transport=private_transport, before_upload=before_upload, verify_only=verify_only, fence_upload=True) def _uploaded_resources(*, root, profile, provider_ref, attachments, cache_dir, resource, - key_field, key_prefix, read_bytes, upload_data, runner, private_transport, + key_field, key_pattern, read_bytes, upload_data, runner, private_transport, before_upload, verify_only, fence_upload=False): keys = [] for attachment in attachments: @@ -72,7 +81,7 @@ def _uploaded_resources(*, root, profile, provider_ref, attachments, cache_dir, with exclusive_file_lock(cache): saved = _read(cache) if cache.exists() else None if saved is not None: - if saved.get("identity") != identity or not re.fullmatch(key_prefix + r"[A-Za-z0-9_-]{1,240}", str(saved.get(key_field, ""))): + if saved.get("identity") != identity or not key_pattern.fullmatch(str(saved.get(key_field, ""))): raise ValueError("resource upload identity conflict or acknowledgement unavailable") keys.append(saved[key_field]) continue @@ -97,7 +106,7 @@ def _uploaded_resources(*, root, profile, provider_ref, attachments, cache_dir, uploaded = _call(args, cwd, runner=runner, private_transport=private_transport) payload = _json_object(uploaded.get("stdout")) key = (payload.get("data") or {}).get(key_field) - if uploaded.get("returncode") != 0 or payload.get("ok") is False or not re.fullmatch(key_prefix + r"[A-Za-z0-9_-]{1,240}", str(key or "")): + if uploaded.get("returncode") != 0 or payload.get("ok") is False or not key_pattern.fullmatch(str(key or "")): raise ValueError("result file upload unavailable") _write(cache, {"identity": identity, key_field: key}) keys.append(key) From 071f9034a842ec87f0db036e973f68a87839746d Mon Sep 17 00:00:00 2001 From: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:51:04 +0800 Subject: [PATCH 2/2] test(lark): pin the acknowledgement boundary the folded patterns own The upload and read-back branches had no direct coverage: the existing result-file fixtures drive the delivery manager and happen to return one valid key, so nothing pinned what an acknowledgement may contain. Fourteen rows state the boundary from the contract instead of from observed output -- one to 240 tail characters accepted, empty tail, 241, a dot, a space, a missing prefix and the wrong resource prefix refused, both branches, the two prefixes staying distinct, and one equality row that pins each folded pattern against the exact string the inline concatenation used to build. Refs #7387 Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com> --- .../extensions/test_lark_upload_key_shape.py | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests/extensions/test_lark_upload_key_shape.py diff --git a/tests/extensions/test_lark_upload_key_shape.py b/tests/extensions/test_lark_upload_key_shape.py new file mode 100644 index 0000000000..133e1a67f4 --- /dev/null +++ b/tests/extensions/test_lark_upload_key_shape.py @@ -0,0 +1,150 @@ +"""The upload acknowledgement shape is one decision, taken the same way twice. + +A Lark upload returns `<1-240 characters of [A-Za-z0-9_-]>`, +and the prefix belongs to the resource kind (`file_` for result files, `img_` +for chat images), not to the caller. Both use sites -- reading a saved +acknowledgement back, and accepting a fresh one -- must answer from the same +pattern, so the boundary cases are pinned here rather than inherited from +whichever value a full delivery fixture happens to return. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from loopx.capabilities.manager_context import _root, _read, _write +from loopx.extensions.lark import return_files + +CACHE_DIR = "lark-result-files" + + +class _Runner: + """Answers the preflight and the create call, and records what was uploaded.""" + + def __init__(self, key: str | None) -> None: + self.key = key + self.creates = 0 + + def __call__(self, args: list[str]) -> dict: + if "--dry-run" in args: + return {"returncode": 0, "stdout": "{}"} + self.creates += 1 + if self.key is None: + return {"returncode": 0, "stdout": json.dumps({"ok": True, "data": {}})} + return { + "returncode": 0, + "stdout": json.dumps({"ok": True, "data": {"file_key": self.key}}), + } + + +def _upload(root: Path, *, key: str | None, pattern=return_files.RESULT_FILE_KEY_PATTERN, + runner: _Runner | None = None) -> tuple[str, _Runner]: + active = runner or _Runner(key) + keys = return_files._uploaded_resources( + root=root, + profile="steward-app", + provider_ref="om_provider", + attachments=[{"sha256": "a" * 64, "name": "result.bin", "size": 4}], + cache_dir=CACHE_DIR, + resource="files", + key_field="file_key", + key_pattern=pattern, + read_bytes=lambda _attachment: b"body", + upload_data=lambda _attachment: {"file_type": "stream", "file_name": "result.bin"}, + runner=active, + private_transport=None, + before_upload=lambda: True, + verify_only=False, + ) + return keys, active + + +def _saved_identity(root: Path) -> tuple[Path, dict]: + identity = { + "profile": "steward-app", + "provider_ref": "om_provider", + "sha256": "a" * 64, + "name": "result.bin", + "size": 4, + } + from loopx.control_plane.collaboration.inbox import _hash + + cache = _root(root) / CACHE_DIR / (_hash(identity) + ".json") + return cache, identity + + +@pytest.mark.parametrize("key", ["file_a", "file_" + "a" * 240, "file_A1-z_9"]) +def test_a_acknowledgement_within_the_shape_is_accepted(tmp_path: Path, key: str) -> None: + keys, runner = _upload(tmp_path, key=key) + + assert keys == (key,) + assert runner.creates == 1 + + +@pytest.mark.parametrize( + "key", + [ + "file_", # the tail is required, an empty body is not a key + "file_" + "a" * 241, # one character past the documented ceiling + "file_a.b", # '.' and ':' are event-id characters, not key ones + "file_a b", + "img_a", # the image prefix cannot answer a file upload + "filea", # the prefix is part of the contract + "", + ], +) +def test_an_acknowledgement_outside_the_shape_is_refused(tmp_path: Path, key: str) -> None: + with pytest.raises(ValueError, match="result file upload unavailable"): + _upload(tmp_path, key=key) + + +def test_the_image_prefix_accepts_what_the_file_prefix_refuses() -> None: + assert return_files.CHAT_IMAGE_KEY_PATTERN.fullmatch("img_a1-Z_9") + assert return_files.RESULT_FILE_KEY_PATTERN.fullmatch("file_a1-Z_9") + assert not return_files.CHAT_IMAGE_KEY_PATTERN.fullmatch("file_a1-Z_9") + assert not return_files.RESULT_FILE_KEY_PATTERN.fullmatch("img_a1-Z_9") + + +def test_a_saved_acknowledgement_outside_the_shape_is_not_reused( + tmp_path: Path, +) -> None: + """The read-back branch asks the same question as the upload branch.""" + + _upload(tmp_path, key="file_stored") + cache, identity = _saved_identity(tmp_path) + _write(cache, {"identity": identity, "file_key": "file_stored!!"}) + + with pytest.raises( + ValueError, match="identity conflict or acknowledgement unavailable" + ): + _upload(tmp_path, key="file_never_used") + + +def test_the_folded_patterns_are_the_construction_they_replace() -> None: + """Folding must not quietly re-decide the shape. + + Each pattern is asserted against the exact string the previous inline + concatenation built, so a changed character class or bound is a review + event rather than a silent widening of what an acknowledgement may contain. + """ + + tail = r"[A-Za-z0-9_-]{1,240}" + assert return_files.RESULT_FILE_KEY_PATTERN.pattern == f"file_{tail}" + assert return_files.CHAT_IMAGE_KEY_PATTERN.pattern == f"img_{tail}" + + +def test_a_saved_acknowledgement_in_shape_is_reused_without_uploading( + tmp_path: Path, +) -> None: + keys, _ = _upload(tmp_path, key="file_stored") + cache, _identity = _saved_identity(tmp_path) + runner = _Runner("file_second_upload") + + reused, active = _upload(tmp_path, key="file_second_upload", runner=runner) + + assert reused == keys + assert active.creates == 0 + assert _read(cache)["file_key"] == "file_stored"