fix(browser): never replace an existing output file with browser_screenshot (#6913)

* fix(browser): never replace an existing output file with browser_screenshot

browser_screenshot published straight onto outputs/<name> through the
replacing atomic publisher, so a screenshot saved as report.png silently
replaced the user's own report.png, an earlier capture, or another
default-named capture taken in the same second (the default stem has
second resolution). The sibling web_capture tool already keeps existing
files and takes a -1, -2, ... suffix.

Stage the complete capture under a hidden sibling and hard-link it onto
the first free candidate name. The link is the whole no-overwrite guard:
it fails with FileExistsError when the name exists as anything, so no
name is checked before it is claimed and a concurrent writer cannot be
replaced; the final name never shows a partial file. Staging shares the
outputs directory, so the link is same-filesystem, and like Gateway
uploads other link failures propagate instead of falling back to
replacement. New files keep the publisher's normal creation mode.

Hidden per-action progress frames keep the replacing _write_screenshot:
their names carry microsecond timestamps in a server-owned directory.

* docs(changelog): reference #6913 for the browser_screenshot no-overwrite fix

* fix(browser): refuse a staged screenshot swapped before it is linked

The sandbox can write outputs/, so it can see the hidden staged capture
and replace it with a symlink before the hard link. os.link follows
symlinks by default, which would publish a hard link to the host file as
the screenshot artifact (the replacing publisher this PR moved away from
never followed one).

Stage the capture with an exclusively created descriptor and keep its
identity from fstat, link without following symlinks where the platform
supports it (decided once at import), and require the published name to
be that inode; anything else is removed and the capture fails, like
Gateway uploads' identity check. A failed staging write still leaves no
files behind.

* docs(readme): note that browser_screenshot never replaces an output file

* fix(browser): keep the staged screenshot open until its link is verified

Linux filesystems reuse a freed inode number at once. Closing the staged
capture before linking let a file swapped in at the staged name take the
same number and pass the samestat check, so the symlink and planted-file
swap tests failed on Linux CI while passing on macOS. Hold the staged
descriptor through the link and identity check, as Gateway uploads do,
and require the published name to be a regular file.

* test(browser): race the peer claim against the real publication step

The claimed-name test patched _publish_bytes_atomically, which the
screenshot path no longer calls, so its hook never fired and the race
assertions passed vacuously. Plant the peer file right before os.link or
os.replace lands bytes on a visible name, and assert that the claim
happened, the capture moved to report-1.png, and the peer file survived.

* refactor(capture): share one output-name candidate sequence

browser_screenshot hand-copied web_capture's probe bound and suffix
order, and the copies had already drifted: Browserless retried a held
timestamped name five times, the screenshot tool once. Move the bound,
the retry count and the candidate order into utils/output_names.py and
have both tools iterate it, so the naming cannot drift again. Browserless
behavior is unchanged; its tests now patch the shared module.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Hyeonsang Cho 2026-10-18 11:52:22 +09:00 committed by GitHub
parent 453563af5b
commit 9b0c7cbc37
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 450 additions and 27 deletions

View File

@ -1247,6 +1247,19 @@ This release closes that milestone with **621 merged pull requests**.
### Fixed
- **browser:** `browser_screenshot` no longer replaces a file that already
exists in the thread's outputs directory. A screenshot saved as
`report.png` used to overwrite the user's own `report.png`, an earlier
capture, or another default-named capture taken in the same second; it now
takes the next free name (`report-1.png`, `report-2.png`, ...) like
`web_capture`, and the tool result reports the name actually used. The
complete capture is hard-linked onto the name, so a name taken concurrently
is skipped rather than replaced and the final name never shows a partial
file. Like Gateway uploads, the outputs directory needs hard-link support;
without it the tool reports an error instead of overwriting. A staged
capture swapped before publication, for example for a symlink to a host
file, is never linked into outputs; the tool reports an error. ([#6913])
- **llm:** A provider error that arrives inside a streaming response that has
already returned `200` is now retried like the same error sent as an HTTP
status. An OpenAI-compatible gateway that answers `200` early, to keep slow
@ -11785,5 +11798,6 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
[#6876]: https://github.com/bytedance/deer-flow/pull/6876
[#6897]: https://github.com/bytedance/deer-flow/pull/6897
[#6909]: https://github.com/bytedance/deer-flow/pull/6909
[#6913]: https://github.com/bytedance/deer-flow/pull/6913
[#6930]: https://github.com/bytedance/deer-flow/pull/6930
[#6991]: https://github.com/bytedance/deer-flow/pull/6991

View File

@ -1002,6 +1002,15 @@
### 修复
- **浏览器:** `browser_screenshot` 不再替换线程 outputs 目录中已存在的文件。此前以
`report.png` 保存的截图会覆盖用户自己的 `report.png`、更早的截图,或同一秒内生成的
另一张默认命名截图;现在会像 `web_capture` 一样改用下一个空闲名称(`report-1.png`、
`report-2.png`……),工具结果会报告实际使用的名称。完整截图通过硬链接发布到目标名称,
因此并发占用的名称会被跳过而不是被替换,最终名称也不会暴露写了一半的文件。与 Gateway
上传相同,outputs 目录需要支持硬链接;不支持时工具会报错,而不会覆盖已有文件。发布前
被替换的暂存截图(例如换成指向宿主机文件的符号链接)绝不会被链接到 outputs,工具会
报错。 ([#6913])
- **llm:** 流式响应已返回 `200` 之后才在流内报告的 provider 错误,现在会像以 HTTP
状态码返回的同一错误一样重试。为了用 SSE keepalive 维持慢请求而提前返回 `200` 的
OpenAI 兼容网关,会把之后发生的 504 作为 SSE `error` 事件发出;openai SDK 将其抛出为
@ -9396,5 +9405,6 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
[#6876]: https://github.com/bytedance/deer-flow/pull/6876
[#6897]: https://github.com/bytedance/deer-flow/pull/6897
[#6909]: https://github.com/bytedance/deer-flow/pull/6909
[#6913]: https://github.com/bytedance/deer-flow/pull/6913
[#6930]: https://github.com/bytedance/deer-flow/pull/6930
[#6991]: https://github.com/bytedance/deer-flow/pull/6991

View File

@ -2707,6 +2707,9 @@ captures from overwriting one another. Failed publication releases the placehold
if cleanup fails, Gateway logs a warning with its path and preserves the original
publication error or interruption.
`browser_screenshot` never replaces an existing `outputs/` file: a taken name
moves to the next free one, such as `report-1.png`.
Browser dependency auto-detection accepts `name`, `group`, and `use` in any
order within a tool entry, with either indented or indentless YAML lists.

View File

@ -59,7 +59,8 @@ storage and transfer cost bounded relative to lossless PNG. The explicit
`browser_screenshot` tool remains PNG because it creates a user-requested
artifact. New automatic capture entry points must reuse the shared progress
encoding definition in `tools.py` so the byte encoding and `.jpg` suffix cannot
drift.
drift. `browser_screenshot` never replaces an `outputs/` file: it hard-links
onto the first free `-N` name.
### Embedded Client (`packages/harness/deerflow/client.py`)

View File

@ -17,12 +17,16 @@ from __future__ import annotations
import asyncio
import contextlib
import errno
import logging
import os
import re
import stat
from dataclasses import dataclass
from datetime import UTC, datetime
from pathlib import Path
from typing import Annotated
from uuid import uuid4
from langchain.tools import InjectedToolCallId, tool
from langchain_core.messages import ToolMessage
@ -35,6 +39,7 @@ from deerflow.config.paths import VIRTUAL_PATH_PREFIX
from deerflow.constants import BROWSER_FRAMES_DIRNAME
from deerflow.tools.types import Runtime
from deerflow.utils.atomic_write import publish_bytes_atomically as _publish_bytes_atomically
from deerflow.utils.output_names import output_name_candidates
from .session import BrowserSession, BrowserSessionManager, PageSnapshot, ScreenshotType, get_browser_session_manager
@ -48,6 +53,8 @@ _OUTPUTS_VIRTUAL_PREFIX = f"{VIRTUAL_PATH_PREFIX}/outputs"
_BROWSER_FRAMES_DIRNAME = BROWSER_FRAMES_DIRNAME
_FRAMES_VIRTUAL_PREFIX = f"{_OUTPUTS_VIRTUAL_PREFIX}/{_BROWSER_FRAMES_DIRNAME}"
_SAFE_FILENAME_RE = re.compile(r"[^A-Za-z0-9._-]+")
# Without no-follow support, os.link may follow a symlink at the staged name.
_LINK_CAN_SKIP_SYMLINKS = os.link in os.supports_follow_symlinks
@dataclass(frozen=True)
@ -440,6 +447,64 @@ def _write_screenshot(outputs_path: Path, name: str, content: bytes) -> str:
return name
def _publish_screenshot_no_overwrite(outputs_path: Path, name: str, content: bytes) -> str:
"""Publish *content* under *name*, or its first free ``-1``, ``-2``, ... variant.
The outputs directory also holds the user's own deliverables, so a
screenshot never replaces an existing file. The complete capture is staged
under a hidden, exclusively created sibling (``0666 & ~umask``, like
``Path.write_bytes``), then hard-linked onto each candidate name: the
``os.link`` is the whole no-overwrite guard, failing with
:class:`FileExistsError` when the name exists as anything (a file, a
symlink, a concurrent capture), so no name is checked before it is claimed.
The final name never shows a partial capture. Staging shares the outputs
directory, so the link is same-filesystem; like Gateway uploads, outputs
need hard-link support and other link failures propagate.
The sandbox can write ``outputs/`` too, so it could swap the staged name for
a symlink to a host file before the link. The link does not follow symlinks
where supported, and the published name must be the regular-file inode
written here; anything else is removed and the capture fails. The staged
descriptor stays open until that check: an unlinked but open inode cannot be
freed, so a swapped-in file can never reuse its number (Linux filesystems
reuse a freed inode number at once) and pass as the capture.
"""
outputs_path.mkdir(parents=True, exist_ok=True)
staged = outputs_path / f".{name}.{uuid4().hex}.tmp"
fd = os.open(staged, os.O_CREAT | os.O_EXCL | os.O_WRONLY | getattr(os, "O_BINARY", 0), 0o666)
try:
try:
handle = os.fdopen(fd, "wb")
except BaseException:
os.close(fd)
raise
with handle:
handle.write(content)
handle.flush()
os.fsync(handle.fileno())
staged_stat = os.fstat(handle.fileno())
for candidate in output_name_candidates(name):
destination = outputs_path / candidate
try:
if _LINK_CAN_SKIP_SYMLINKS:
os.link(staged, destination, follow_symlinks=False)
else:
os.link(staged, destination)
except FileExistsError:
continue
published = os.lstat(destination)
if not stat.S_ISREG(published.st_mode) or not os.path.samestat(published, staged_stat):
destination.unlink(missing_ok=True)
raise OSError(f"Staged browser screenshot changed before publication: {staged.name}")
return candidate
raise FileExistsError(errno.EEXIST, "No free screenshot name", str(outputs_path / name))
finally:
try:
staged.unlink(missing_ok=True)
except OSError:
logger.warning("Failed to remove staged browser screenshot: %s", staged, exc_info=True)
@tool("browser_screenshot", parse_docstring=True)
async def browser_screenshot_tool(
runtime: Runtime,
@ -454,7 +519,7 @@ async def browser_screenshot_tool(
stateless page load.
Args:
filename: Optional output filename (extension is forced to .png).
filename: Optional output filename (extension is forced to .png). An existing file is never replaced; the screenshot takes the next free name such as report-1.png.
full_page: Capture the full scrollable page instead of just the viewport.
"""
try:
@ -464,7 +529,7 @@ async def browser_screenshot_tool(
with _resolve_session(runtime, "browser_screenshot") as session:
content = await session.screenshot_bytes(full_page=full_page)
name = _safe_screenshot_name(filename)
final_name = await asyncio.to_thread(_write_screenshot, outputs_path, name, content)
final_name = await asyncio.to_thread(_publish_screenshot_no_overwrite, outputs_path, name, content)
virtual_path = f"{_OUTPUTS_VIRTUAL_PREFIX}/{final_name}"
return Command(
update={

View File

@ -17,6 +17,7 @@ from deerflow.config import get_app_config
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
from deerflow.tools.types import Runtime
from deerflow.utils.atomic_write import publish_bytes_atomically as _publish_bytes_atomically
from deerflow.utils.output_names import output_name_candidates
from deerflow.utils.readability import ReadabilityExtractor
from .browserless_client import BrowserlessClient, BrowserlessFetchResult, BrowserlessScreenshotResult
@ -32,11 +33,6 @@ _OUTPUT_FORMAT_TO_EXTENSION = {
"webp": "webp",
}
_SAFE_FILENAME_RE = re.compile(r"[^A-Za-z0-9._-]+")
# Cap collision-suffix probing so a saturated outputs directory cannot spin forever.
_MAX_FILENAME_COLLISION_PROBES = 1000
# Retry bound for the timestamped fallback: the microseconds stamp can only repeat
# when another writer claimed that exact name within the same microsecond.
_MAX_TIMESTAMP_CLAIM_ATTEMPTS = 5
DEFAULT_BASE_URL = "http://localhost:3032"
_UNSET = object()
@ -257,21 +253,9 @@ def _dedupe_output_name(outputs_path: Path, output_name: str) -> str:
means another writer is holding them; returning a name anyway would
reintroduce the silent overwrite this function exists to prevent.
"""
stem = Path(output_name).stem
suffix = Path(output_name).suffix
if _reserve_output_name(outputs_path, output_name):
return output_name
for index in range(1, _MAX_FILENAME_COLLISION_PROBES + 1):
probe = f"{stem}-{index}{suffix}"
if _reserve_output_name(outputs_path, probe):
return probe
for _ in range(_MAX_TIMESTAMP_CLAIM_ATTEMPTS):
timestamp = datetime.now(UTC).strftime("%Y%m%d-%H%M%S-%f")
fallback = f"{stem}-{timestamp}{suffix}"
if _reserve_output_name(outputs_path, fallback):
return fallback
for candidate in output_name_candidates(output_name):
if _reserve_output_name(outputs_path, candidate):
return candidate
raise RuntimeError(f"could not reserve an output filename for {output_name!r} in {outputs_path}")

View File

@ -6,6 +6,8 @@ capture/fetch publication. It writes and fsyncs a hidden exclusive sibling befor
umask; replacements preserve existing permission bits before publication. Never
read the process umask by changing it in this threaded runtime. Behavioral tests
for both tool imports live in `tests/test_browser_outputs_atomic.py`.
Both tools take collision names from `output_names.output_name_candidates`
(name, `-1`..`-1000`, five timestamps); claim each atomically, never pre-check.
### Model Retry Hints

View File

@ -0,0 +1,28 @@
"""Collision-free output file names shared by the capture tools."""
from __future__ import annotations
from collections.abc import Iterator
from datetime import UTC, datetime
from pathlib import Path
# Cap collision-suffix probing so a saturated outputs directory cannot spin forever.
MAX_COLLISION_PROBES = 1000
# Retry bound for the timestamped fallback: the microseconds stamp can only repeat
# when another writer claimed that exact name within the same microsecond.
MAX_TIMESTAMP_ATTEMPTS = 5
def output_name_candidates(name: str) -> Iterator[str]:
"""Yield *name*, then ``stem-1`` .. ``stem-1000``, then timestamped fallbacks.
Callers claim each candidate atomically and move on when it is taken; the
generator never checks the filesystem, so naming cannot race the claim.
Timestamps are taken lazily, one per fallback attempt.
"""
yield name
stem, suffix = Path(name).stem, Path(name).suffix
for index in range(1, MAX_COLLISION_PROBES + 1):
yield f"{stem}-{index}{suffix}"
for _ in range(MAX_TIMESTAMP_ATTEMPTS):
yield f"{stem}-{datetime.now(UTC).strftime('%Y%m%d-%H%M%S-%f')}{suffix}"

View File

@ -0,0 +1,237 @@
# SPDX-License-Identifier: MIT
"""``browser_screenshot`` must never replace a file already in the outputs directory."""
from __future__ import annotations
import errno
import os
import stat
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from deerflow.community.browser_automation import tools
_SHOT = b"\x89PNG\r\n\x1a\nscreenshot"
async def _screenshot(outputs: Path, filename: str | None = "report.png"):
session = MagicMock()
session.screenshot_bytes = AsyncMock(return_value=_SHOT)
manager = MagicMock()
manager.get_session.return_value = session
runtime = SimpleNamespace(context={"thread_id": "thread-1"}, state={"thread_data": {"outputs_path": str(outputs)}})
with patch.object(tools, "get_browser_session_manager", return_value=manager), patch.object(tools, "_get_tool_config", return_value={}):
return await tools.browser_screenshot_tool.coroutine(runtime=runtime, tool_call_id="t1", filename=filename)
def _artifact(result) -> str:
return result.update["artifacts"][0]
@pytest.mark.asyncio
async def test_screenshot_keeps_an_existing_output_and_takes_the_next_name(tmp_path):
(tmp_path / "report.png").write_bytes(b"user report")
result = await _screenshot(tmp_path)
assert _artifact(result) == "/mnt/user-data/outputs/report-1.png"
assert "Saved browser screenshot: /mnt/user-data/outputs/report-1.png" in result.update["messages"][0].content
assert (tmp_path / "report.png").read_bytes() == b"user report"
assert (tmp_path / "report-1.png").read_bytes() == _SHOT
assert sorted(path.name for path in tmp_path.iterdir()) == ["report-1.png", "report.png"]
@pytest.mark.asyncio
async def test_repeated_screenshots_with_one_filename_keep_every_capture(tmp_path):
names = [_artifact(await _screenshot(tmp_path)).rsplit("/", 1)[1] for _ in range(3)]
assert names == ["report.png", "report-1.png", "report-2.png"]
assert sorted(path.name for path in tmp_path.iterdir()) == sorted(names)
@pytest.mark.asyncio
async def test_a_name_claimed_after_the_choice_is_not_replaced(tmp_path, monkeypatch):
# A peer (another tool call, or the user) can create a name after the
# publisher chose it. Simulate that window at the publication step itself:
# right before a link or replace lands the capture on a visible name, a
# peer claims that name. Only a publication that refuses occupied names at
# that step keeps the peer file.
link, replace = os.link, os.replace
claimed: list[Path] = []
def peer_claims(destination) -> None:
destination = Path(destination)
if not claimed and not destination.name.startswith("."):
destination.write_bytes(b"peer output")
claimed.append(destination)
def link_after_peer(source, destination, *args, **kwargs):
peer_claims(destination)
return link(source, destination, *args, **kwargs)
def replace_after_peer(source, destination, *args, **kwargs):
peer_claims(destination)
return replace(source, destination, *args, **kwargs)
monkeypatch.setattr(os, "link", link_after_peer)
monkeypatch.setattr(os, "replace", replace_after_peer)
result = await _screenshot(tmp_path)
assert claimed == [tmp_path / "report.png"]
assert _artifact(result) == "/mnt/user-data/outputs/report-1.png"
assert (tmp_path / "report.png").read_bytes() == b"peer output"
assert (tmp_path / "report-1.png").read_bytes() == _SHOT
@pytest.mark.skipif(os.name == "nt", reason="POSIX symlink creation")
@pytest.mark.asyncio
async def test_screenshot_does_not_replace_a_symlink_at_the_name(tmp_path):
outputs = tmp_path / "outputs"
outputs.mkdir()
(outputs / "report.png").symlink_to(tmp_path / "missing-target.png")
result = await _screenshot(outputs)
assert _artifact(result) == "/mnt/user-data/outputs/report-1.png"
assert (outputs / "report.png").is_symlink()
assert not (tmp_path / "missing-target.png").exists()
@pytest.mark.asyncio
async def test_failed_publication_reports_an_error_and_leaves_no_files(tmp_path, monkeypatch):
(tmp_path / "report.png").write_bytes(b"user report")
def no_hard_links(*args, **kwargs):
raise OSError(errno.EPERM, "hard links unsupported")
monkeypatch.setattr(os, "link", no_hard_links)
result = await _screenshot(tmp_path)
assert "artifacts" not in result.update
assert "Error: could not capture screenshot" in result.update["messages"][0].content
assert sorted(path.name for path in tmp_path.iterdir()) == ["report.png"]
assert (tmp_path / "report.png").read_bytes() == b"user report"
def _swap_staged_before_link(monkeypatch, replace_staged, watch: Path | None = None) -> list[int]:
"""Replace the staged capture right before it is linked, as a sandbox writing ``outputs/`` could.
Returns the link counts of *watch* observed right after each link call.
"""
link = os.link
link_counts: list[int] = []
def swapped_link(source, destination, *args, **kwargs):
source = Path(source)
if source.name.startswith(".") and source.is_file() and not source.is_symlink():
source.unlink()
replace_staged(source)
try:
return link(source, destination, *args, **kwargs)
finally:
if watch is not None:
link_counts.append(watch.stat().st_nlink)
monkeypatch.setattr(os, "link", swapped_link)
return link_counts
@pytest.mark.skipif(os.name == "nt", reason="POSIX symlink creation")
@pytest.mark.asyncio
async def test_a_staged_capture_swapped_for_a_symlink_never_publishes_its_target(tmp_path, monkeypatch):
outputs = tmp_path / "outputs"
outputs.mkdir()
secret = tmp_path / "host-secret.txt"
secret.write_bytes(b"host secret")
link_counts = _swap_staged_before_link(monkeypatch, lambda staged: staged.symlink_to(secret), watch=secret)
result = await _screenshot(outputs)
assert "artifacts" not in result.update
assert "Error: could not capture screenshot" in result.update["messages"][0].content
assert list(outputs.iterdir()) == []
# The host file never gains a name in outputs/, not even briefly.
assert link_counts and set(link_counts) == {1}
@pytest.mark.asyncio
async def test_a_staged_capture_swapped_for_another_file_is_not_published(tmp_path, monkeypatch):
_swap_staged_before_link(monkeypatch, lambda staged: staged.write_bytes(b"planted"))
result = await _screenshot(tmp_path)
assert "artifacts" not in result.update
assert "Error: could not capture screenshot" in result.update["messages"][0].content
assert list(tmp_path.iterdir()) == []
def _open_descriptors_for(target: os.stat_result) -> list[int]:
descriptors = []
for entry in os.listdir("/dev/fd"):
try:
if os.path.samestat(os.fstat(int(entry)), target):
descriptors.append(int(entry))
except OSError:
continue
return descriptors
@pytest.mark.skipif(not os.path.isdir("/dev/fd"), reason="needs /dev/fd to list open descriptors")
@pytest.mark.asyncio
async def test_the_staged_capture_stays_open_while_it_is_linked(tmp_path, monkeypatch):
# Linux filesystems hand a freed inode number to the next file at once, so
# a capture whose descriptor is closed before the link lets a swapped-in
# file reuse its number and pass the identity check. macOS rarely reuses
# numbers, so the swap tests above only catch that there; pin the guard
# itself: an open descriptor keeps the staged inode allocated.
link = os.link
open_at_link: list[list[int]] = []
def observing_link(source, destination, *args, **kwargs):
open_at_link.append(_open_descriptors_for(os.lstat(source)))
return link(source, destination, *args, **kwargs)
monkeypatch.setattr(os, "link", observing_link)
result = await _screenshot(tmp_path)
assert _artifact(result) == "/mnt/user-data/outputs/report.png"
assert open_at_link and all(open_at_link)
@pytest.mark.asyncio
async def test_a_failed_staging_write_leaves_no_files(tmp_path, monkeypatch):
def fail(*args):
raise OSError(errno.ENOSPC, "disk full")
monkeypatch.setattr(os, "fsync", fail)
result = await _screenshot(tmp_path)
assert "Error: could not capture screenshot" in result.update["messages"][0].content
assert list(tmp_path.iterdir()) == []
@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits and umask")
@pytest.mark.parametrize("mask", [0o022, 0o077])
@pytest.mark.asyncio
async def test_screenshot_uses_normal_creation_permissions(tmp_path, mask):
previous_mask = os.umask(mask)
try:
await _screenshot(tmp_path)
finally:
os.umask(previous_mask)
assert stat.S_IMODE((tmp_path / "report.png").stat().st_mode) == 0o666 & ~mask
@pytest.mark.asyncio
async def test_default_names_from_the_same_second_do_not_collide(tmp_path, monkeypatch):
monkeypatch.setattr(tools, "_safe_screenshot_name", lambda filename: "browser-capture-20261015-120000.png")
first = _artifact(await _screenshot(tmp_path, filename=None))
second = _artifact(await _screenshot(tmp_path, filename=None))
assert first.endswith("/browser-capture-20261015-120000.png")
assert second.endswith("/browser-capture-20261015-120000-1.png")

View File

@ -22,6 +22,7 @@ from datetime import UTC, datetime
import pytest
tools = importlib.import_module("deerflow.community.browserless.tools")
output_names = importlib.import_module("deerflow.utils.output_names")
@pytest.fixture()
@ -152,7 +153,7 @@ def test_interruption_propagates_and_frees_the_name(outputs, monkeypatch):
def test_probe_bound_is_inclusive(outputs, monkeypatch):
"""The last probe index is reachable, so a directory full of ``-1``/``-2`` still gets ``-3``."""
monkeypatch.setattr(tools, "_MAX_FILENAME_COLLISION_PROBES", 3)
monkeypatch.setattr(output_names, "MAX_COLLISION_PROBES", 3)
for name in ("report.png", "report-1.png", "report-2.png"):
(outputs / name).write_bytes(b"earlier")
@ -160,7 +161,7 @@ def test_probe_bound_is_inclusive(outputs, monkeypatch):
def test_saturated_probe_range_still_reserves_a_fresh_name(outputs, monkeypatch):
monkeypatch.setattr(tools, "_MAX_FILENAME_COLLISION_PROBES", 2)
monkeypatch.setattr(output_names, "MAX_COLLISION_PROBES", 2)
taken = ["report.png", "report-1.png", "report-2.png", "report-3.png"]
for name in taken:
(outputs / name).write_bytes(b"earlier")
@ -185,8 +186,8 @@ def test_exhausted_timestamp_claims_refuse_to_overwrite(outputs, monkeypatch):
def now(cls, tz=None):
return datetime(2026, 10, 14, 12, 0, 0, 123456, tzinfo=UTC)
monkeypatch.setattr(tools, "_MAX_FILENAME_COLLISION_PROBES", 1)
monkeypatch.setattr(tools, "datetime", _FrozenClock)
monkeypatch.setattr(output_names, "MAX_COLLISION_PROBES", 1)
monkeypatch.setattr(output_names, "datetime", _FrozenClock)
held = ["report.png", "report-1.png", "report-20261014-120000-123456.png"]
for name in held:
(outputs / name).write_bytes(b"earlier")

View File

@ -0,0 +1,78 @@
# SPDX-License-Identifier: MIT
"""Both capture tools take output names from one shared candidate sequence."""
from __future__ import annotations
import importlib
from datetime import UTC, datetime
from itertools import islice
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from deerflow.utils import output_names
browser_automation_tools = importlib.import_module("deerflow.community.browser_automation.tools")
browserless_tools = importlib.import_module("deerflow.community.browserless.tools")
class _StepClock(datetime):
"""Each call advances one microsecond, so every fallback stamp is distinct."""
ticks = 0
@classmethod
def now(cls, tz=None):
cls.ticks += 1
return datetime(2026, 10, 18, 12, 0, 0, cls.ticks, tzinfo=UTC)
def test_candidates_keep_the_name_then_number_then_timestamp(monkeypatch):
monkeypatch.setattr(output_names, "MAX_COLLISION_PROBES", 2)
monkeypatch.setattr(_StepClock, "ticks", 0)
monkeypatch.setattr(output_names, "datetime", _StepClock)
assert list(output_names.output_name_candidates("report.png")) == [
"report.png",
"report-1.png",
"report-2.png",
*(f"report-20261018-120000-{tick:06d}.png" for tick in range(1, output_names.MAX_TIMESTAMP_ATTEMPTS + 1)),
]
def test_candidates_read_the_clock_only_when_a_fallback_is_reached(monkeypatch):
monkeypatch.setattr(_StepClock, "ticks", 0)
monkeypatch.setattr(output_names, "datetime", _StepClock)
assert len(list(islice(output_names.output_name_candidates("a.png"), output_names.MAX_COLLISION_PROBES + 1))) == output_names.MAX_COLLISION_PROBES + 1
assert _StepClock.ticks == 0
def test_both_capture_tools_share_the_candidate_sequence():
for module in (browser_automation_tools, browserless_tools):
assert module.output_name_candidates is output_names.output_name_candidates
@pytest.mark.asyncio
async def test_screenshot_retries_a_held_timestamp_fallback(tmp_path, monkeypatch):
# Browserless already retried a held timestamped name; the screenshot tool
# now shares that bound instead of giving up after one stamp.
monkeypatch.setattr(output_names, "MAX_COLLISION_PROBES", 1)
monkeypatch.setattr(_StepClock, "ticks", 0)
monkeypatch.setattr(output_names, "datetime", _StepClock)
held = ["report.png", "report-1.png", "report-20261018-120000-000001.png"]
for name in held:
(tmp_path / name).write_bytes(b"earlier")
session = MagicMock()
session.screenshot_bytes = AsyncMock(return_value=b"shot")
manager = MagicMock()
manager.get_session.return_value = session
runtime = SimpleNamespace(context={"thread_id": "thread-1"}, state={"thread_data": {"outputs_path": str(tmp_path)}})
with patch.object(browser_automation_tools, "get_browser_session_manager", return_value=manager), patch.object(browser_automation_tools, "_get_tool_config", return_value={}):
result = await browser_automation_tools.browser_screenshot_tool.coroutine(runtime=runtime, tool_call_id="t1", filename="report.png")
assert result.update["artifacts"] == ["/mnt/user-data/outputs/report-20261018-120000-000002.png"]
assert (tmp_path / "report-20261018-120000-000002.png").read_bytes() == b"shot"
assert all((tmp_path / name).read_bytes() == b"earlier" for name in held)