diff --git a/backend/packages/harness/deerflow/uploads/manager.py b/backend/packages/harness/deerflow/uploads/manager.py index debef97233..a4d4247378 100644 --- a/backend/packages/harness/deerflow/uploads/manager.py +++ b/backend/packages/harness/deerflow/uploads/manager.py @@ -96,7 +96,7 @@ def _fit_utf8_bytes(text: str, budget: int) -> str: def claim_unique_filename(name: str, seen: set[str]) -> str: - """Generate a unique filename by appending ``_N`` suffix on collision. + """Generate a case-insensitively unique filename by appending ``_N`` on collision. Automatically adds the returned name to *seen* so callers don't need to. @@ -112,9 +112,10 @@ def claim_unique_filename(name: str, seen: set[str]) -> str: seen: Set of filenames already claimed (mutated in place). Returns: - A filename not present in *seen* (already added to *seen*). + A filename not present in *seen* even ignoring case (already added to *seen*). """ - if name not in seen: + claimed = {existing.casefold() for existing in seen} + if name.casefold() not in claimed: seen.add(name) return name stem, suffix = Path(name).stem, Path(name).suffix @@ -128,7 +129,7 @@ def claim_unique_filename(name: str, seen: set[str]) -> str: candidate = _fit_utf8_bytes(stem + suffix, _MAX_FILENAME_BYTES - len(tag.encode("utf-8"))) + tag else: candidate = f"{_fit_utf8_bytes(stem, budget)}{tag}{suffix}" - if candidate not in seen: + if candidate.casefold() not in claimed: break counter += 1 seen.add(candidate) diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 8483b64724..048fc459d9 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -2,6 +2,10 @@ Backend tests must preserve the runtime invariants they exercise without changing production execution topology. +Upload case-collision coverage uses separate HTTP requests and preserves both +reported payloads. Observe real filename-claim inputs to pin the disk seed; +case-insensitive hosts can otherwise mask a missing seed through link retries. + Channel reload cancellation regressions must assert the worker returned the stale snapshot before checking that newer runtime config survived. Completion alone cannot prove the race: loader exceptions are caught and return `None`. diff --git a/backend/tests/test_channel_file_attachments.py b/backend/tests/test_channel_file_attachments.py index d1d9cf77fe..22ecb4443f 100644 --- a/backend/tests/test_channel_file_attachments.py +++ b/backend/tests/test_channel_file_attachments.py @@ -282,6 +282,29 @@ class TestResolveAttachments: class TestInboundFileIngestion: + def test_case_variant_names_keep_both_inbound_attachments(self, tmp_path): + from app.channels import manager + + uploads_dir = tmp_path / "uploads" + uploads_dir.mkdir() + msg = InboundMessage( + channel_name="telegram", + chat_id="chat-1", + user_id="user-1", + text="see attachments", + files=[ + {"type": "file", "filename": "Report.txt", "_content": b"first"}, + {"type": "file", "filename": "report.txt", "_content": b"second"}, + ], + ) + + with patch("deerflow.uploads.manager.ensure_uploads_dir", return_value=uploads_dir): + result = _run(manager._ingest_inbound_files("thread-1", msg)) + + assert [item["filename"] for item in result] == ["Report.txt", "report_1.txt"] + assert (uploads_dir / "Report.txt").read_bytes() == b"first" + assert (uploads_dir / "report_1.txt").read_bytes() == b"second" + def test_consumes_inline_channel_bytes_without_exposing_them_downstream(self, tmp_path): from app.channels import manager diff --git a/backend/tests/test_uploads_manager.py b/backend/tests/test_uploads_manager.py index 54326fe98d..10c8e3806f 100644 --- a/backend/tests/test_uploads_manager.py +++ b/backend/tests/test_uploads_manager.py @@ -136,6 +136,11 @@ class TestDeduplicateFilename: assert claim_unique_filename("data.txt", seen) == "data_1.txt" assert "data_1.txt" in seen + def test_case_insensitive_collision_including_suffix(self): + seen = {"Report.txt", "report_1.TXT"} + assert claim_unique_filename("report.txt", seen) == "report_2.txt" + assert "report_2.txt" in seen + def test_triple_collision(self): seen = {"data.txt", "data_1.txt", "data_2.txt"} assert claim_unique_filename("data.txt", seen) == "data_3.txt" diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 390bc740e8..11b723fcf7 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -16,6 +16,7 @@ from fastapi import HTTPException, UploadFile from fastapi.testclient import TestClient from app.gateway import upload_ingestion +from app.gateway.auth.models import User from app.gateway.deps import get_config from app.gateway.routers import uploads from deerflow.sandbox.lease import get_sandbox_lease_manager @@ -141,6 +142,46 @@ def test_upload_files_auto_renames_duplicate_form_filenames(tmp_path): assert (thread_uploads_dir / "data_1.txt").read_bytes() == b"second" +def test_upload_files_deduplicates_case_variants_across_requests(tmp_path): + thread_uploads_dir = tmp_path / "uploads" + thread_uploads_dir.mkdir() + user = User(email="upload-test@example.com") + app = make_authed_test_app(user_factory=lambda: user, bind_current_user=True) + app.include_router(uploads.router) + app.dependency_overrides[get_config] = lambda: SimpleNamespace() + + claims = [] + real_claim = uploads.claim_unique_filename + + def capture_claim(name, seen): + claims.append((name, set(seen))) + return real_claim(name, seen) + + with ( + patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir), + patch.object(uploads, "get_sandbox_provider", return_value=_mounted_provider()), + patch.object(uploads, "claim_unique_filename", side_effect=capture_claim), + TestClient(app) as client, + ): + first = client.post("/api/threads/thread-local/uploads", files={"files": ("Report.txt", b"first")}) + assert first.status_code == 200 + assert first.json()["success"] is True + assert [entry["filename"] for entry in first.json()["files"]] == ["Report.txt"] + + second = client.post("/api/threads/thread-local/uploads", files={"files": ("report.txt", b"second")}) + + assert second.status_code == 200 + assert second.json()["success"] is True + assert [entry["filename"] for entry in second.json()["files"]] == ["report_1.txt"] + assert second.json()["files"][0]["original_filename"] == "report.txt" + assert (thread_uploads_dir / first.json()["files"][0]["filename"]).read_bytes() == b"first" + assert (thread_uploads_dir / second.json()["files"][0]["filename"]).read_bytes() == b"second" + assert {path.name for path in thread_uploads_dir.iterdir()} == {"Report.txt", "report_1.txt"} + # On case-insensitive hosts the link retry can hide a missing disk seed. + # Observe the real claim inputs so this invariant is checked on every host. + assert claims == [("Report.txt", set()), ("report.txt", {"Report.txt"})] + + def test_upload_files_deduplicates_max_length_filenames_without_failing_the_batch(tmp_path): # A 255-byte filename is the longest normalize_filename accepts. Before # the byte-budget truncation in claim_unique_filename, deduplicating a