3 Commits

Author SHA1 Message Date
PerryLink
d04ee1d131
fix(utils):Keep the timestamped output-name fallbacks distinct on a coarse clock (#7071)
* fix(harness): keep timestamped output-name fallbacks distinct

`output_name_candidates()` sampled `datetime.now(UTC)` inside the yield
loop, so its five timestamped fallbacks were only distinct when the host
clock advanced between two generator steps.  On a coarse clock (Windows
advances in sub-millisecond steps) all five produced the same name, and
the capture tools lost retries before failing with
`could not reserve an output filename`.

Sample the clock once when the first fallback is reached and offset each
attempt by one microsecond, so distinctness no longer depends on the host
clock.  Add a regression test that freezes the clock, and update the
exhaustion test to hold all five distinct stamps.

* docs: describe the coarse clock accurately

---------

Co-authored-by: PerryLink <255665900+PerryLink@users.noreply.github.com>
2026-10-25 16:17:15 +08:00
Hyeonsang Cho
9b0c7cbc37
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>
2026-10-18 10:52:22 +08:00
Dragon ze
2768b86fff
fix(community): reserve Browserless capture names before publishing (#6901)
* fix(community): reserve Browserless capture names before publishing

Name selection and publication were two separate operations:
`_dedupe_output_name` probed with `Path.exists()` and then
`publish_bytes_atomically` ran `os.replace`, so two `web_capture` calls in the
same thread outputs directory could both settle on `report.png` before either
wrote, and the second publication replaced the first capture while both reported
success for the same artifact path (#6883). `web_capture_tool` runs the writer
through `asyncio.to_thread`, and same-second default names collide the same way
because the generated stem's timestamp has second resolution.

Claim the name instead of checking for it: an `O_CREAT | O_EXCL` open either
creates an empty placeholder or reports the name held, so the probe walk
(-1, -2, ... then the microsecond timestamp) is race-free. `_write_capture_output`
releases the placeholder when publication fails, catching BaseException so an
interruption also frees the name, and a failing unlink is logged rather than
allowed to mask the original error - the same error-masking family #6871 is
addressing in the shared publisher, which this change deliberately does not
touch.

When even the timestamped names are held, the function now raises instead of
returning a name it cannot own; returning one would reintroduce the silent
overwrite. `tests/test_browser_outputs_atomic.py` pins the publisher call as
source text, so its expectation moved to the new call shape in the same commit.

Refs #6883

* fix(community): open the reserved capture name with new-file permissions

An `os.open` without an explicit mode creates the file with `0o777 & ~umask`, and
`publish_bytes_atomically` preserves an existing destination's mode, so the
reservation added in the previous commit made every capture executable (`0o755`
under the usual umask) where main produced `0o666 & ~umask` (`0o644`) through the
publisher's new-file branch. Pass `0o666` explicitly and pin the mode against a
`Path.write_bytes` reference in the same directory, which is the permission the
publisher documents for new files.

Refs #6883

* fix(browserless): warn when capture reservation cleanup fails

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-10-15 12:44:09 +08:00