mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(artifacts): invalidate digests after external file replacement (#6195)
* fix(artifacts): invalidate digests after external file replacement * docs(gateway): keep artifact guidance within the module budget * fix(artifacts): validate oversized ranges by file identity --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Guo-Yixin
GitHub
parent
261279ff80
commit
68014de157
@ -1873,6 +1873,13 @@ After each run, DeerFlow records a workspace change summary for the run-owned `w
|
||||
|
||||
Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. When a completed response successfully presents between 2 and 50 files, its final file card also offers one ZIP download. Archive membership comes from the terminal delivery receipt rather than browser-supplied paths, and the ZIP contains the current file versions, which may have changed since the response. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel on Unix and Windows while the thread is idle; saves use content revisions to prevent overwriting agent changes. Source previews also recognize extensionless `Dockerfile` and `Makefile` artifacts by their file names. Unknown file types, including names such as `constructor` and `__proto__`, retain the download fallback.
|
||||
|
||||
Artifact content revisions refresh when an output file is atomically replaced,
|
||||
even if its size and modification time are preserved. Refreshing the preview
|
||||
then provides the new revision for saving; an older preview still requires a reload.
|
||||
Regular files over the 2 MiB editing limit use file identity and change metadata
|
||||
for range validators without hashing the whole file. Conditional byte ranges for
|
||||
regular files require a matching ETag; date-form `If-Range` requests receive the full current file.
|
||||
|
||||
CSV and TSV artifacts open as tables in the artifact panel and in a separate window. The preview preserves text values (including leading zeros), supports an optional header row, and pages through up to 200 rows and 50 columns from the initial sample. Long or multiline cells can be opened and copied in full. Switch to source to inspect or edit the file; downloads and separate windows use the saved version.
|
||||
|
||||
If the sample cuts a CRLF line ending in half, the preview keeps the earlier complete rows and omits the incomplete final record, including when its last field is quoted.
|
||||
|
||||
@ -119,7 +119,7 @@ owner-scoped assistant version selection remains enabled.
|
||||
| **Memory** (`/api/memory`) | `GET /` - memory data; `POST /reload` - force reload; `GET /config` - config; `GET /status` - config + data |
|
||||
| **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload files (auto-converts PDF/PPT/Excel/Word); non-mounted sync releases its lease after the last holder exits; `GET /list` - list; `DELETE /{filename}` - delete a regular file; symlinks 404 and a converted `.md` is kept (#5672) |
|
||||
| **Threads** (`/api/threads/{id}`) | `DELETE /` - remove DeerFlow-managed local thread data after LangGraph thread deletion; `POST /branches` - branch a completed assistant turn with a replay checkpoint; inherited titles take next-free displayed sibling suffixes, including explicit/renamed ones, while explicit titles stay unchanged. Durable `branch` admission rejects races. Workspace files are not checkpointed, so the branch only best-effort copies the current workspace when branching from the **latest** turn (`workspace_clone_mode="current_thread_best_effort"`); branching from an older/historical turn skips the copy (`workspace_clone_mode="skipped_historical_turn"`) so the branch never inherits files that only exist in a later timeline. Thread-scoped runtime channels (`sandbox`, `thread_data`) are not copied onto the branch: the parent's `sandbox_id` binds path mappings and the release lifecycle to the parent's workspace, so the branch lazily acquires its own sandbox instead. Branch creation also seeds the new thread's run-event feed from the branch checkpoint's visible messages (`history_seed_mode` in the response): the thread feed reads run_events, not checkpoints, so without the seed the inherited history disappears from the UI after the branch's first run (#4380). Seeded rows are grouped into one synthetic run per inherited turn (`branch-seed-{thread_id}-{n}`, a new turn opening at every persisted human message, including an allowlisted hidden `ask_clarification` reply) because `run_id` is a turn identity to the feed's consumers, not a provenance tag: regenerating an inherited answer supersedes that row's whole `run_id` in `GET /messages/page`, so one shared id for the entire seed deleted the complete inherited history on a branch's first regenerate (#4458); `GET /goal`, `PUT /goal`, `DELETE /goal` - read, set, and clear the active thread goal; `POST /compact` - summarize older active context, deriving memory policy and bucket from the state-producing checkpoint rather than request `agent_name`, and block while a run is in flight; unexpected failures return a generic 500 detail |
|
||||
| **Artifacts** (`/api/threads/{id}/artifacts`) | `GET /{path}` streams text/binary artifacts with byte-`Range` support; active HTML/XML/XSL/`+xml` content (including `.skill` members) is forced to download, and `?download=true` forces other types. `PUT /{path}` atomically replaces an existing UTF-8 file under `/mnt/user-data/outputs` on matching SHA-256; active runs conflict. Non-mounted sandbox sync uses a request lease, and caller cancellation drains the remote/local commit-or-rollback before releasing the thread-operation reservation or lease. `path_utils.resolve_outputs_confined_path` normalizes `..` and re-checks the resolved host path under the outputs root, so encoded traversal or output symlinks cannot reach sibling uploads. POSIX ownership/mode is preserved where descriptor APIs exist; Windows keeps native temp-file permissions. |
|
||||
| **Artifacts** (`/api/threads/{id}/artifacts`) | GET streams byte ranges; HTML/XML/XSL/`+xml` (also `.skill` members) and `?download=true` force attachments. PUT atomically replaces existing UTF-8 outputs on matching SHA-256; active runs conflict. Non-mounted sync uses a request lease and drains remote/local commit-or-rollback across cancellation before releasing reservations/leases. `resolve_outputs_confined_path` normalizes `..` and confines host paths against traversal/symlinks. Preserve POSIX ownership/mode where supported; Windows keeps temp-file permissions. Digest cache: path/dev/ino/ctime_ns/mtime_ns/size; identity invalidates same-size/mtime replacements. Large regular files use stat-prefixed ETags; If-Range requires matching ETags. Windows ctime is creation time. Keep cache reuse, size limits and worker hashing. |
|
||||
| **Suggestions** (`/api/suggestions`) | `GET /config` - returns global suggestions config boolean; `POST /threads/{id}/suggestions` - generate follow-up questions; rich list/block model content is normalized and inline reasoning (`<think>...</think>`, including unclosed/truncated blocks from reasoning models like MiniMax-M3) is stripped before JSON parsing |
|
||||
| **Input Polish** (`/api/input-polish`) | `POST /` - rewrite a composer draft before it is sent. This is a short authenticated `runs:create` LLM request using `input_polish` config; it does not create a LangGraph run, persist a message, or modify thread state. Shares the non-graph one-shot LLM path (`deerflow.utils.oneshot_llm.run_oneshot_llm`) with the suggestions route so model build + Langfuse metadata + invoke stay in one place; validates the same stripped view of the draft it sends to the model, and preserves literal `<think>` substrings in the rewrite (`strip_think_blocks(truncate_unclosed=False)`) |
|
||||
| **Thread Runs** (`/api/threads/{id}/runs`) | `POST /` - create background run; `POST /stream` - create + SSE stream; `POST /wait` - create + block. Before the first journaled run, seed an empty feed from a checkpoint so legacy checkpoint-only history keeps its order and visibility; skip absent checkpoints or populated feeds. `POST /regenerate/prepare` - prepare clean input + checkpoint metadata for regenerating the latest completed or interrupted assistant answer, treating a protocol-valid hidden `human_input_response` as confirmed replay input while rejecting other hidden/control humans, preserving that response's hidden/correlation metadata, and carrying the latest non-empty thread title in graph input so resuming an older checkpoint cannot roll back a later manual rename (#4457); `POST /edit-regenerate/prepare` - prepare a checkpoint replay from the latest editable human turn with a replacement user message and edit replay metadata; it carries the current thread title the same way, but only when the replay base already has one — an untitled base belongs to a thread the title middleware has not named yet, so pinning the current title there would keep a name generated from the prompt the edit just replaced; `GET /` - newest 100 runs as an array; `GET /page` - keyset history page `{data, has_more, next_before_created_at, next_before_run_id}`; `GET /{rid}` - run details; `POST /{rid}/cancel` - cancel; `GET /{rid}/join` - join SSE; `GET /{rid}/stream` hides action/wait; GET action 405 pre-owner; POST needs `runs:cancel`; `GET /{rid}/messages` - paginated per-run messages `{data, has_more}`; `GET /{rid}/events` - full event stream; `GET /{rid}/workspace-changes` - workspace/output file change summary and optional diffs; `GET/POST /{rid}/artifacts/archive` - receipt manifest / bounded ZIP; `GET /../messages` - legacy thread message array; `GET /../messages/page` - backward thread-global `seq` history page with middleware/subagent-AI/successful-regenerate/edit-replay filtering and page-run-scoped feedback enrichment; subagent AI callbacks remain available through run events while parent `task` ToolMessages stay visible for card restoration; `GET /../token-usage` - aggregate tokens plus an optional `context_usage` percentage. Context usage approximately counts messages from the latest materialized thread state through `build_thread_checkpoint_state_accessor`, so full and delta checkpoint modes expose the same input. The percentage uses the latest run's model and its `context_window`. |
|
||||
|
||||
@ -341,17 +341,27 @@ def _sha256_of_file(path: Path) -> str:
|
||||
(e.g. http://<lan-ip>:<port>) and otherwise breaks artifact preview +
|
||||
inline editing (see issue #4864).
|
||||
|
||||
The digest is cached by (path, mtime_ns, size) so the many small ``Range``
|
||||
The digest is cached by path and file identity/change metadata so small ``Range``
|
||||
requests a browser issues while scrubbing/paginating a preview do not each
|
||||
re-hash a potentially huge artifact from scratch (raised in PR review).
|
||||
"""
|
||||
stat = path.stat()
|
||||
return _sha256_of_file_cached(str(path), stat.st_mtime_ns, stat.st_size)
|
||||
# Sandbox syncs and other writers can atomically replace a same-size file
|
||||
# while preserving its mtime. Identity separates those generations; ctime
|
||||
# also invalidates in-place writes where the filesystem exposes change time.
|
||||
return _sha256_of_file_cached(
|
||||
str(path),
|
||||
stat.st_dev,
|
||||
stat.st_ino,
|
||||
stat.st_ctime_ns,
|
||||
stat.st_mtime_ns,
|
||||
stat.st_size,
|
||||
)
|
||||
|
||||
|
||||
@functools.lru_cache(maxsize=256)
|
||||
def _sha256_of_file_cached(path: str, mtime_ns: int, size: int) -> str:
|
||||
"""Cached SHA-256 of *path*; the size/mtime args invalidate stale entries."""
|
||||
def _sha256_of_file_cached(path: str, device: int, inode: int, ctime_ns: int, mtime_ns: int, size: int) -> str:
|
||||
"""Cached SHA-256 of *path*; metadata arguments separate file generations."""
|
||||
digest = hashlib.sha256()
|
||||
with open(path, "rb") as handle:
|
||||
for chunk in iter(lambda: handle.read(1 << 20), b""):
|
||||
@ -359,6 +369,27 @@ def _sha256_of_file_cached(path: str, mtime_ns: int, size: int) -> str:
|
||||
return digest.hexdigest()
|
||||
|
||||
|
||||
def _artifact_etag(path: Path) -> str:
|
||||
"""Keep content revisions for editable files and metadata-only tags for large files."""
|
||||
file_stat = path.stat()
|
||||
if file_stat.st_size <= MAX_EDITABLE_ARTIFACT_BYTES:
|
||||
return f'"{_sha256_of_file(path)}"'
|
||||
|
||||
# Do not read oversized artifacts just to validate a small byte range.
|
||||
# The prefix keeps this opaque validator distinct from an editable SHA-256.
|
||||
identity = f"{file_stat.st_dev}:{file_stat.st_ino}:{file_stat.st_ctime_ns}:{file_stat.st_mtime_ns}:{file_stat.st_size}"
|
||||
return f'"stat-{hashlib.sha256(identity.encode("ascii")).hexdigest()}"'
|
||||
|
||||
|
||||
class _ArtifactFileResponse(FileResponse):
|
||||
"""Require an ETag to authorize conditional ranges of replaceable artifacts."""
|
||||
|
||||
def _should_use_range(self, http_if_range: str) -> bool:
|
||||
# Sandbox syncs can preserve mtimes across replacements, so Last-Modified
|
||||
# is not a strong validator. Date-form If-Range must return the full file.
|
||||
return http_if_range == self.headers["etag"]
|
||||
|
||||
|
||||
@router.get(
|
||||
"/threads/{thread_id}/artifacts/{path:path}",
|
||||
summary="Get Artifact File",
|
||||
@ -470,15 +501,8 @@ async def get_artifact(thread_id: ThreadId, path: str, request: Request, downloa
|
||||
# Always force download for active content types to prevent script
|
||||
# execution in the application origin when users open generated artifacts.
|
||||
headers = {**_build_attachment_headers(actual_path.name)}
|
||||
file_size = await asyncio.to_thread(lambda: actual_path.stat().st_size)
|
||||
if file_size <= MAX_EDITABLE_ARTIFACT_BYTES:
|
||||
# Real SHA-256 so the browser can skip crypto.subtle (unavailable
|
||||
# on non-secure contexts) when previewing / editing artifacts (#4864).
|
||||
# Skipped for oversized artifacts to avoid a full-file read on every
|
||||
# GET / Range request (raised in review as a performance P1).
|
||||
content_sha256 = await asyncio.to_thread(_sha256_of_file, actual_path)
|
||||
headers["ETag"] = f'"{content_sha256}"'
|
||||
return FileResponse(
|
||||
headers["ETag"] = await asyncio.to_thread(_artifact_etag, actual_path)
|
||||
return _ArtifactFileResponse(
|
||||
path=actual_path,
|
||||
filename=actual_path.name,
|
||||
media_type=mime_type,
|
||||
@ -488,15 +512,8 @@ async def get_artifact(thread_id: ThreadId, path: str, request: Request, downloa
|
||||
if kind == "inline_file":
|
||||
# FileResponse honors byte-Range requests for large text previews and
|
||||
headers = {"Content-Disposition": _build_content_disposition("inline", actual_path.name), "X-Content-Type-Options": "nosniff"}
|
||||
file_size = await asyncio.to_thread(lambda: actual_path.stat().st_size)
|
||||
if file_size <= MAX_EDITABLE_ARTIFACT_BYTES:
|
||||
# Real SHA-256 so the browser can skip crypto.subtle (unavailable
|
||||
# on non-secure contexts) when previewing / editing artifacts (#4864).
|
||||
# Skipped for oversized artifacts to avoid a full-file read on every
|
||||
# GET / Range request (raised in review as a performance P1).
|
||||
content_sha256 = await asyncio.to_thread(_sha256_of_file, actual_path)
|
||||
headers["ETag"] = f'"{content_sha256}"'
|
||||
return FileResponse(
|
||||
headers["ETag"] = await asyncio.to_thread(_artifact_etag, actual_path)
|
||||
return _ArtifactFileResponse(
|
||||
path=actual_path,
|
||||
media_type=mime_type,
|
||||
headers=headers,
|
||||
|
||||
@ -132,3 +132,20 @@ async def test_update_artifact_does_not_block_event_loop(tmp_path: Path, monkeyp
|
||||
|
||||
assert result.sha256 == hashlib.sha256(b"updated").hexdigest()
|
||||
assert await asyncio.to_thread(target.read_bytes) == b"updated"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("filename", ["large.txt", "large.html", "large.bin"])
|
||||
@pytest.mark.parametrize("download", [False, True])
|
||||
async def test_large_artifact_validator_does_not_block_event_loop(tmp_path: Path, monkeypatch, filename: str, download: bool) -> None:
|
||||
vpath = f"mnt/user-data/outputs/{filename}"
|
||||
target = await _seed(tmp_path, monkeypatch, "t1", vpath)
|
||||
await asyncio.to_thread(target.write_bytes, b"a" * (artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES + 1))
|
||||
|
||||
def unexpected_hash(*_args, **_kwargs):
|
||||
pytest.fail("Large artifact response planning must not hash the file")
|
||||
|
||||
monkeypatch.setattr(artifacts_router, "_sha256_of_file", unexpected_hash)
|
||||
response = await _get_artifact("t1", vpath, request=None, download=download)
|
||||
|
||||
assert isinstance(response, FileResponse)
|
||||
assert response.headers["etag"].startswith('"stat-')
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
import asyncio
|
||||
import hashlib
|
||||
import os
|
||||
import stat
|
||||
import threading
|
||||
import zipfile
|
||||
@ -616,6 +617,147 @@ def test_get_artifact_inline_text_returns_sha256_etag(tmp_path, monkeypatch) ->
|
||||
assert response.headers.get("etag") == f'"{expected}"'
|
||||
|
||||
|
||||
def _replace_preserving_artifact_mtime(path: Path, content: bytes) -> None:
|
||||
before = path.stat()
|
||||
staged = path.with_name("replacement.part")
|
||||
staged.write_bytes(content)
|
||||
os.utime(staged, ns=(before.st_atime_ns, before.st_mtime_ns))
|
||||
os.replace(staged, path)
|
||||
after = path.stat()
|
||||
assert after.st_size == before.st_size
|
||||
assert after.st_mtime_ns == before.st_mtime_ns
|
||||
assert after.st_ino != before.st_ino
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("filename", "query", "request_headers", "disposition", "expected_status"),
|
||||
[
|
||||
("note.txt", "", {}, "inline", 200),
|
||||
("note.txt", "", {"Range": "bytes=0-2"}, "inline", 206),
|
||||
("note.txt", "?download=true", {}, "attachment", 200),
|
||||
("page.html", "", {}, "attachment", 200),
|
||||
],
|
||||
)
|
||||
def test_artifact_etag_tracks_external_atomic_replacement(tmp_path, monkeypatch, filename, query, request_headers, disposition, expected_status) -> None:
|
||||
artifact_path = tmp_path / filename
|
||||
old, new = b"old report", b"new report"
|
||||
artifact_path.write_bytes(old)
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = f"/api/threads/thread-1/artifacts/mnt/user-data/outputs/{filename}{query}"
|
||||
|
||||
with TestClient(app) as client:
|
||||
first = client.get(url)
|
||||
assert first.content == old
|
||||
assert first.headers["etag"] == f'"{hashlib.sha256(old).hexdigest()}"'
|
||||
_replace_preserving_artifact_mtime(artifact_path, new)
|
||||
second = client.get(url, headers=request_headers)
|
||||
|
||||
assert second.status_code == expected_status
|
||||
assert second.content == (new[:3] if request_headers else new)
|
||||
assert second.headers["etag"] == f'"{hashlib.sha256(new).hexdigest()}"'
|
||||
assert second.headers["content-disposition"].startswith(f"{disposition};")
|
||||
if request_headers:
|
||||
assert second.headers["content-range"] == f"bytes 0-2/{len(new)}"
|
||||
|
||||
|
||||
def test_refreshed_artifact_etag_can_be_used_to_save(tmp_path, monkeypatch) -> None:
|
||||
artifact_path = tmp_path / "note.txt"
|
||||
artifact_path.write_bytes(b"old report")
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
_patch_artifact_update_dependencies(monkeypatch, artifact_path)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = "/api/threads/thread-1/artifacts/mnt/user-data/outputs/note.txt"
|
||||
|
||||
with TestClient(app) as client:
|
||||
previous = client.get(url)
|
||||
_replace_preserving_artifact_mtime(artifact_path, b"new report")
|
||||
refreshed = client.get(url)
|
||||
rejected = client.put(url, json={"content": "user edit", "expected_sha256": previous.headers["etag"].strip('"')})
|
||||
saved = client.put(url, json={"content": "user edit", "expected_sha256": refreshed.headers["etag"].strip('"')})
|
||||
|
||||
assert rejected.status_code == 412
|
||||
assert saved.status_code == 200
|
||||
assert artifact_path.read_bytes() == b"user edit"
|
||||
assert saved.json()["sha256"] == hashlib.sha256(b"user edit").hexdigest()
|
||||
|
||||
|
||||
def test_replaced_artifact_rejects_previous_if_range_validator(tmp_path, monkeypatch) -> None:
|
||||
artifact_path = tmp_path / "note.txt"
|
||||
artifact_path.write_bytes(b"old report")
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = "/api/threads/thread-1/artifacts/mnt/user-data/outputs/note.txt"
|
||||
|
||||
with TestClient(app) as client:
|
||||
first = client.get(url)
|
||||
_replace_preserving_artifact_mtime(artifact_path, b"new report")
|
||||
response = client.get(url, headers={"Range": "bytes=0-2", "If-Range": first.headers["etag"]})
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.content == b"new report"
|
||||
assert "content-range" not in response.headers
|
||||
assert response.headers["etag"] == f'"{hashlib.sha256(b"new report").hexdigest()}"'
|
||||
|
||||
|
||||
@pytest.mark.parametrize("changed_field", ["st_dev", "st_ino", "st_ctime_ns"])
|
||||
def test_artifact_digest_invalidates_when_file_identity_changes(tmp_path, monkeypatch, changed_field) -> None:
|
||||
artifact_path = tmp_path / "note.txt"
|
||||
artifact_path.write_bytes(b"old report")
|
||||
actual_stat = artifact_path.stat()
|
||||
metadata = {field: getattr(actual_stat, field) for field in ("st_dev", "st_ino", "st_ctime_ns", "st_mtime_ns", "st_size")}
|
||||
original_stat = Path.stat
|
||||
|
||||
def controlled_stat(path, *args, **kwargs):
|
||||
return SimpleNamespace(**metadata) if path == artifact_path else original_stat(path, *args, **kwargs)
|
||||
|
||||
# Control the metadata rather than depending on filesystem clock resolution
|
||||
# or treating Windows creation time as a POSIX change time.
|
||||
monkeypatch.setattr(Path, "stat", controlled_stat)
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
assert artifacts_router._sha256_of_file(artifact_path) == hashlib.sha256(b"old report").hexdigest()
|
||||
artifact_path.write_bytes(b"new report")
|
||||
metadata[changed_field] += 1
|
||||
assert artifacts_router._sha256_of_file(artifact_path) == hashlib.sha256(b"new report").hexdigest()
|
||||
|
||||
|
||||
def test_unchanged_artifact_digest_is_reused_without_rereading(tmp_path, monkeypatch) -> None:
|
||||
artifact_path = tmp_path / "note.txt"
|
||||
artifact_path.write_bytes(b"unchanged report")
|
||||
expected = hashlib.sha256(b"unchanged report").hexdigest()
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
assert artifacts_router._sha256_of_file(artifact_path) == expected
|
||||
|
||||
def unexpected_open(*_args, **_kwargs):
|
||||
pytest.fail("An unchanged artifact must not be rehashed for each preview")
|
||||
|
||||
monkeypatch.setattr(artifacts_router, "open", unexpected_open, raising=False)
|
||||
assert artifacts_router._sha256_of_file(artifact_path) == expected
|
||||
|
||||
|
||||
def test_artifact_digest_cache_separates_paths_with_matching_metadata(tmp_path, monkeypatch) -> None:
|
||||
paths = [tmp_path / "first.txt", tmp_path / "second.txt"]
|
||||
contents = [b"old report", b"new report"]
|
||||
for path, content in zip(paths, contents, strict=True):
|
||||
path.write_bytes(content)
|
||||
metadata = SimpleNamespace(st_dev=1, st_ino=2, st_ctime_ns=3, st_mtime_ns=4, st_size=10)
|
||||
original_stat = Path.stat
|
||||
|
||||
def controlled_stat(path, *args, **kwargs):
|
||||
return metadata if path in paths else original_stat(path, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(Path, "stat", controlled_stat)
|
||||
artifacts_router._sha256_of_file_cached.cache_clear()
|
||||
for path, content in zip(paths, contents, strict=True):
|
||||
assert artifacts_router._sha256_of_file(path) == hashlib.sha256(content).hexdigest()
|
||||
|
||||
|
||||
def test_get_skill_archive_preview_supports_bounded_range_requests(tmp_path, monkeypatch) -> None:
|
||||
payload = ("skill preview \u4e2d\u6587\n" * 100_000).encode()
|
||||
skill_path = tmp_path / "sample.skill"
|
||||
@ -951,11 +1093,10 @@ def test_skill_archive_preview_rejects_oversized_member_before_decompression(tmp
|
||||
assert exc_info.value.status_code == 413
|
||||
|
||||
|
||||
def test_get_artifact_large_text_skips_etag(tmp_path, monkeypatch) -> None:
|
||||
def test_get_artifact_large_text_skips_content_hashing(tmp_path, monkeypatch) -> None:
|
||||
# A text artifact larger than MAX_EDITABLE_ARTIFACT_BYTES must not be hashed
|
||||
# on every GET / Range request (performance P1 from review). The response
|
||||
# still streams, but carries no full-content SHA-256 ETag; the client falls
|
||||
# back to its own hashing where crypto.subtle is available.
|
||||
# still streams with an opaque metadata validator, not an editable digest.
|
||||
payload = b"a" * (artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES + 1)
|
||||
artifact_path = tmp_path / "large.txt"
|
||||
artifact_path.write_bytes(payload)
|
||||
@ -973,15 +1114,11 @@ def test_get_artifact_large_text_skips_etag(tmp_path, monkeypatch) -> None:
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
# Oversized artifacts skip the full-file SHA-256 pass, so there must be no
|
||||
# 64-hex content-hash ETag. Starlette's FileResponse may still attach a
|
||||
# cheap mtime/size-derived ETag, which requires no file read.
|
||||
etag = response.headers.get("etag")
|
||||
assert etag is None or len(etag.strip('"')) != 64
|
||||
assert response.headers["etag"].startswith('"stat-')
|
||||
assert response.content == payload
|
||||
|
||||
|
||||
def test_get_artifact_large_active_content_skips_etag(tmp_path, monkeypatch) -> None:
|
||||
def test_get_artifact_large_active_content_skips_content_hashing(tmp_path, monkeypatch) -> None:
|
||||
# Active content (e.g. .html) is force-downloaded. A large active file must
|
||||
# still force a download but skip the full-file SHA-256 pass (performance P1).
|
||||
payload = "<html>" + ("a" * (artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES + 1)) + "</html>"
|
||||
@ -1004,4 +1141,127 @@ def test_get_artifact_large_active_content_skips_etag(tmp_path, monkeypatch) ->
|
||||
|
||||
assert isinstance(response, FileResponse)
|
||||
assert response.headers.get("content-disposition", "").startswith("attachment;")
|
||||
assert response.headers.get("etag") is None
|
||||
assert response.headers["etag"].startswith('"stat-')
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("filename", "query", "disposition"),
|
||||
[
|
||||
("large.txt", "", "inline"),
|
||||
("large.txt", "?download=true", "attachment"),
|
||||
("large.html", "", "attachment"),
|
||||
("large.bin", "", "inline"),
|
||||
],
|
||||
)
|
||||
@pytest.mark.parametrize("validator_header", ["etag", "last-modified"])
|
||||
def test_large_artifact_replacement_rejects_stale_if_range(tmp_path, monkeypatch, filename, query, disposition, validator_header) -> None:
|
||||
artifact_path = tmp_path / filename
|
||||
prefix = b"\x00" if filename.endswith(".bin") else b"a"
|
||||
old = prefix + b"a" * artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES
|
||||
new = prefix + b"b" * artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES
|
||||
artifact_path.write_bytes(old)
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
|
||||
def unexpected_hash(*_args, **_kwargs):
|
||||
pytest.fail("Oversized artifacts must not be read in full to generate an ETag")
|
||||
|
||||
monkeypatch.setattr(artifacts_router, "_sha256_of_file", unexpected_hash)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = f"/api/threads/thread-1/artifacts/mnt/user-data/outputs/{filename}{query}"
|
||||
|
||||
with TestClient(app) as client:
|
||||
first = client.get(url)
|
||||
assert first.content == old
|
||||
matching = client.get(url, headers={"Range": "bytes=0-2", "If-Range": first.headers["etag"]})
|
||||
assert matching.status_code == 206
|
||||
assert matching.content == old[:3]
|
||||
_replace_preserving_artifact_mtime(artifact_path, new)
|
||||
stale = client.get(url, headers={"Range": "bytes=0-2", "If-Range": first.headers[validator_header]})
|
||||
assert stale.status_code == 200
|
||||
assert stale.content == new
|
||||
assert "content-range" not in stale.headers
|
||||
assert stale.headers["etag"] != first.headers["etag"]
|
||||
assert stale.headers["last-modified"] == first.headers["last-modified"]
|
||||
assert stale.headers["etag"].startswith('"stat-')
|
||||
assert stale.headers["content-disposition"].startswith(f"{disposition};")
|
||||
current = client.get(url, headers={"Range": "bytes=0-2", "If-Range": stale.headers["etag"]})
|
||||
ordinary = client.get(url, headers={"Range": "bytes=0-2"})
|
||||
|
||||
for response in (current, ordinary):
|
||||
assert response.status_code == 206
|
||||
assert response.content == new[:3]
|
||||
assert response.headers["content-range"] == f"bytes 0-2/{len(new)}"
|
||||
assert response.headers["etag"] == stale.headers["etag"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("changed_field", ["st_dev", "st_ino", "st_ctime_ns", "st_mtime_ns", "st_size"])
|
||||
@pytest.mark.parametrize("download", [False, True])
|
||||
def test_large_artifact_validator_tracks_metadata_without_reading(tmp_path, monkeypatch, changed_field, download) -> None:
|
||||
artifact_path = tmp_path / "large.txt"
|
||||
artifact_path.write_bytes(b"a" * (artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES + 1))
|
||||
file_stat = artifact_path.stat()
|
||||
metadata = {field: getattr(file_stat, field) for field in ("st_dev", "st_ino", "st_ctime_ns", "st_mtime_ns", "st_size", "st_mode", "st_mtime")}
|
||||
original_stat = Path.stat
|
||||
original_open = Path.open
|
||||
|
||||
def controlled_stat(path, *args, **kwargs):
|
||||
return SimpleNamespace(**metadata) if path == artifact_path else original_stat(path, *args, **kwargs)
|
||||
|
||||
def unexpected_open(path, *args, **kwargs):
|
||||
if path == artifact_path:
|
||||
pytest.fail("Planning an oversized text response must only read metadata")
|
||||
return original_open(path, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(Path, "stat", controlled_stat)
|
||||
monkeypatch.setattr(Path, "open", unexpected_open)
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
|
||||
def plan_response():
|
||||
return asyncio.run(call_unwrapped(artifacts_router.get_artifact, "thread-1", "mnt/user-data/outputs/large.txt", _make_request(), download=download))
|
||||
|
||||
first = plan_response()
|
||||
unchanged = plan_response()
|
||||
assert first.headers["etag"] == unchanged.headers["etag"]
|
||||
assert first.headers["etag"].startswith('"stat-')
|
||||
metadata[changed_field] += 1
|
||||
changed = plan_response()
|
||||
assert changed.headers["etag"] != first.headers["etag"]
|
||||
|
||||
|
||||
def test_small_artifact_replacement_rejects_date_if_range(tmp_path, monkeypatch) -> None:
|
||||
artifact_path = tmp_path / "note.txt"
|
||||
artifact_path.write_bytes(b"old report")
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = "/api/threads/thread-1/artifacts/mnt/user-data/outputs/note.txt"
|
||||
|
||||
with TestClient(app) as client:
|
||||
first = client.get(url)
|
||||
_replace_preserving_artifact_mtime(artifact_path, b"new report")
|
||||
response = client.get(url, headers={"Range": "bytes=0-2", "If-Range": first.headers["last-modified"]})
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.content == b"new report"
|
||||
assert "content-range" not in response.headers
|
||||
assert response.headers["etag"] == f'"{hashlib.sha256(b"new report").hexdigest()}"'
|
||||
|
||||
|
||||
def test_artifact_at_editing_size_limit_keeps_a_saveable_content_revision(tmp_path, monkeypatch) -> None:
|
||||
artifact_path = tmp_path / "limit.txt"
|
||||
payload = b"a" * artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES
|
||||
artifact_path.write_bytes(payload)
|
||||
monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path)
|
||||
_patch_artifact_update_dependencies(monkeypatch, artifact_path)
|
||||
app = make_authed_test_app()
|
||||
app.include_router(artifacts_router.router)
|
||||
url = "/api/threads/thread-1/artifacts/mnt/user-data/outputs/limit.txt"
|
||||
|
||||
with TestClient(app) as client:
|
||||
preview = client.get(url)
|
||||
assert preview.headers["etag"] == f'"{hashlib.sha256(payload).hexdigest()}"'
|
||||
saved = client.put(url, json={"content": "edited", "expected_sha256": preview.headers["etag"].strip('"')})
|
||||
|
||||
assert saved.status_code == 200
|
||||
assert artifact_path.read_bytes() == b"edited"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user