fix(browser): publish captured screenshots and fetches atomically (#6731)

* fix(browser): publish captured screenshots and fetches atomically

_write_screenshot and _write_capture_output wrote the final path with a truncating
write and then returned its name, so a failure part-way through left a partial
capture that the caller reported as saved - and the leftovers then made the dedupe
helper pick a new name for the next capture. Publish through a temporary file plus
os.replace.

* fix(browser): preserve permissions on atomic capture publication

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

README.md

@@ -2439,6 +2439,11 @@ This is the difference between a chatbot with tool access and an agent with an a
 
 ### Agentic Browser Control
 
+Browser screenshots and Browserless capture/fetch outputs are published only
+after the complete file is written. New files use the host's normal creation
+permissions (typically `0644`), and replacements preserve existing permissions,
+so sandbox code can read captures when those permissions allow it.
+
 Browser dependency auto-detection accepts `name`, `group`, and `use` in any
 order within a tool entry, with either indented or indentless YAML lists.
 

backend/packages/harness/deerflow/community/browser_automation/tools.py

@@ -34,6 +34,7 @@ from deerflow.config import get_app_config
 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 .session import BrowserSession, BrowserSessionManager, PageSnapshot, ScreenshotType, get_browser_session_manager
 
@@ -435,7 +436,7 @@ def _thread_outputs_path(runtime: Runtime) -> Path | str:
 
 def _write_screenshot(outputs_path: Path, name: str, content: bytes) -> str:
     outputs_path.mkdir(parents=True, exist_ok=True)
-    (outputs_path / name).write_bytes(content)
+    _publish_bytes_atomically(outputs_path / name, content)
     return name
 
 

backend/packages/harness/deerflow/community/browserless/tools.py

@@ -16,6 +16,7 @@ from deerflow.community.url_safety import validate_delegated_backend_url, valida
 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.readability import ReadabilityExtractor
 
 from .browserless_client import BrowserlessClient, BrowserlessFetchResult, BrowserlessScreenshotResult
@@ -247,7 +248,7 @@ def _write_capture_output(outputs_path: Path, output_name: str, content: bytes)
     """Write ``content`` into ``outputs_path`` and return the actual filename used."""
     outputs_path.mkdir(parents=True, exist_ok=True)
     final_name = _dedupe_output_name(outputs_path, output_name)
-    (outputs_path / final_name).write_bytes(content)
+    _publish_bytes_atomically(outputs_path / final_name, content)
     return final_name
 
 

backend/packages/harness/deerflow/utils/AGENTS.md

@@ -1,3 +1,12 @@
+### Atomic Byte Publication
+
+`atomic_write.publish_bytes_atomically` owns browser screenshot and Browserless
+capture/fetch publication. It writes and fsyncs a hidden exclusive sibling before
+`os.replace`, cleaning it on failure. New files use `0666` filtered by the host
+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`.
+
 ### Model Retry Hints
 
 `retry_after.py::bounded_retry_after_ms` shares the 24-hour provider-hint ceiling

backend/packages/harness/deerflow/utils/atomic_write.py

@@ -0,0 +1,46 @@
+"""Atomic byte publication with normal file-creation permissions."""
+
+from __future__ import annotations
+
+import os
+import stat
+from pathlib import Path
+from uuid import uuid4
+
+
+def publish_bytes_atomically(path: Path, content: bytes) -> None:
+    """Replace *path* with complete bytes, preserving its existing mode.
+
+    New files use ``0666 & ~umask``, like ``Path.write_bytes``, so captures
+    remain readable through sandbox mounts under the usual ``0022`` umask.
+    A hidden, exclusively created sibling keeps partial writes away from the
+    final path. Failure before replacement leaves the previous file intact.
+    """
+    try:
+        mode = stat.S_IMODE(path.stat().st_mode)
+    except FileNotFoundError:
+        mode = None
+
+    temporary = path.with_name(f".{path.name}.{uuid4().hex}.tmp")
+    flags = os.O_CREAT | os.O_EXCL | os.O_WRONLY | getattr(os, "O_BINARY", 0)
+    # Let the kernel apply umask; reading it via os.umask would mutate
+    # process-global state while other threads may be creating files.
+    fd = os.open(temporary, flags, 0o666)
+    try:
+        handle = os.fdopen(fd, "wb")
+        fd = None  # The file object now owns the descriptor.
+        with handle:
+            handle.write(content)
+            handle.flush()
+            if mode is not None:
+                if os.name == "nt":
+                    os.chmod(temporary, mode)
+                else:
+                    os.fchmod(handle.fileno(), mode)
+            os.fsync(handle.fileno())
+        os.replace(temporary, path)
+    except BaseException:
+        if fd is not None:
+            os.close(fd)
+        temporary.unlink(missing_ok=True)
+        raise

backend/tests/test_browser_outputs_atomic.py

@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: MIT
+"""Browser captures must not be published half written under the final name."""
+
+from __future__ import annotations
+
+import inspect
+import os
+import stat
+from pathlib import Path
+
+import pytest
+
+from deerflow.community.browser_automation import tools as browser_automation_tools
+from deerflow.community.browserless import tools as browserless_tools
+from deerflow.utils.atomic_write import publish_bytes_atomically
+
+
+@pytest.fixture(params=[browser_automation_tools, browserless_tools], ids=["browser", "browserless"])
+def publisher(request):
+    return request.param._publish_bytes_atomically
+
+
+def test_publish_replaces_only_after_content_is_complete(tmp_path, monkeypatch, publisher):
+    target = tmp_path / "shot.png"
+    target.write_bytes(b"old")
+    replace = os.replace
+    publications = []
+
+    def observe_replace(source, destination):
+        assert target.read_bytes() == b"old"
+        assert destination == target
+        temporary = Path(source)
+        assert temporary.parent == target.parent
+        assert temporary.name.startswith(".")
+        assert temporary.read_bytes() == b"new complete capture"
+        if os.name != "nt":
+            assert stat.S_IMODE(temporary.stat().st_mode) == stat.S_IMODE(target.stat().st_mode)
+        publications.append(destination)
+        replace(source, destination)
+
+    monkeypatch.setattr(os, "replace", observe_replace)
+    publisher(target, b"new complete capture")
+
+    assert publications == [target]
+    assert target.read_bytes() == b"new complete capture"
+    assert list(tmp_path.iterdir()) == [target]
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits and umask")
+@pytest.mark.parametrize("mode", [0o600, 0o640, 0o644])
+def test_publish_preserves_existing_permissions(tmp_path, publisher, mode):
+    target = tmp_path / "shot.png"
+    target.write_bytes(b"old")
+    target.chmod(mode)
+
+    publisher(target, b"new")
+
+    assert target.read_bytes() == b"new"
+    assert stat.S_IMODE(target.stat().st_mode) == mode
+    assert list(tmp_path.iterdir()) == [target]
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits and umask")
+@pytest.mark.parametrize("mask", [0o022, 0o077])
+def test_publish_new_file_uses_normal_creation_permissions(tmp_path, publisher, mask):
+    target = tmp_path / "shot.png"
+    previous_mask = os.umask(mask)
+    try:
+        publisher(target, b"new")
+    finally:
+        os.umask(previous_mask)
+
+    assert target.read_bytes() == b"new"
+    assert stat.S_IMODE(target.stat().st_mode) == 0o666 & ~mask
+    assert list(tmp_path.iterdir()) == [target]
+
+
+@pytest.mark.parametrize("stage", ["write", "fsync", "replace"])
+@pytest.mark.parametrize("existing", [False, True], ids=["new", "existing"])
+def test_publish_failure_preserves_target_and_cleans_temporary(tmp_path, monkeypatch, publisher, stage, existing):
+    target = tmp_path / "shot.png"
+    if existing:
+        target.write_bytes(b"old")
+    old_mode = target.stat().st_mode if existing else None
+
+    def fail(*args, **kwargs):
+        raise OSError("publication failed")
+
+    if stage == "write":
+        fdopen = os.fdopen
+
+        class PartialWriter:
+            def __init__(self, handle):
+                self.handle = handle
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                self.handle.close()
+
+            def write(self, content):
+                self.handle.write(content[:2])
+                fail()
+
+        monkeypatch.setattr(os, "fdopen", lambda *args, **kwargs: PartialWriter(fdopen(*args, **kwargs)))
+    else:
+        monkeypatch.setattr(os, stage, fail)
+
+    with pytest.raises(OSError, match="publication failed"):
+        publisher(target, b"new complete capture")
+
+    if existing:
+        assert target.read_bytes() == b"old"
+        assert target.stat().st_mode == old_mode
+        assert list(tmp_path.iterdir()) == [target]
+    else:
+        assert not target.exists()
+        assert list(tmp_path.iterdir()) == []
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX descriptor permissions")
+def test_publish_permission_failure_keeps_previous_file(tmp_path, monkeypatch, publisher):
+    target = tmp_path / "shot.png"
+    target.write_bytes(b"old")
+    target.chmod(0o640)
+
+    def fail(*args):
+        raise OSError("permission setup failed")
+
+    monkeypatch.setattr(os, "fchmod", fail)
+    with pytest.raises(OSError, match="permission setup failed"):
+        publisher(target, b"new")
+
+    assert target.read_bytes() == b"old"
+    assert stat.S_IMODE(target.stat().st_mode) == 0o640
+    assert list(tmp_path.iterdir()) == [target]
+
+
+def test_screenshot_writes_go_through_the_atomic_publish():
+    source = inspect.getsource(browser_automation_tools)
+
+    # A truncating write left a partial capture behind while the function returned
+    # the name as if it had been saved.
+    assert "(outputs_path / name).write_bytes(" not in source
+    assert "_publish_bytes_atomically(outputs_path / name, content)" in source
+
+
+def test_capture_writes_go_through_the_atomic_publish():
+    source = inspect.getsource(browserless_tools)
+
+    assert "(outputs_path / final_name).write_bytes(" not in source
+    assert "_publish_bytes_atomically(outputs_path / final_name, content)" in source
+
+
+def test_both_tools_share_the_atomic_publisher():
+    for module in (browser_automation_tools, browserless_tools):
+        assert module._publish_bytes_atomically is publish_bytes_atomically