Skip to content

feat(vtz): screenshot::fetcher local-probe path — Task 3a [#2865]#2874

Merged
viniciusdacal merged 1 commit intomainfrom
feat/phase-1-task-03-fetcher
Apr 20, 2026
Merged

feat(vtz): screenshot::fetcher local-probe path — Task 3a [#2865]#2874
viniciusdacal merged 1 commit intomainfrom
feat/phase-1-task-03-fetcher

Conversation

@matheuspoleza
Copy link
Copy Markdown
Contributor

Summary

Task 3a of Phase 1 (plans/2865-phase-1-headless-screenshot.md) — network-free half of the Chrome binary resolver. Ships a stable resolve_local_chrome API so Task 4 (pool) can start depending on it. The download + cache-manifest half (with wiremock dep) is Task 3b.

Why the split

Task 3 as scoped in the design doc covers both local probing and network download. Shipping it whole blocks Task 4 on something like a week of interleaved work across env/syspath probes + Chrome for Testing JSON + SHA-256 verify + $HOME fallback + macOS quarantine. Splitting at the natural seam (resolver vs downloader) keeps each PR reviewable and unblocks Task 4 sooner against the local-probe path that covers the 95% case (developer has Chrome installed).

Public API (this PR)

pub const SYSTEM_CHROME_PATHS: &[&str] = &[/* macOS, Linux */];

pub fn resolve_local_chrome(
    env_path: Option<&str>,
    probe_paths: &[&str],
) -> Option<PathBuf>;

Resolution order:

  1. $VERTZ_CHROME_PATH (passed by caller) if executable
  2. First executable match in SYSTEM_CHROME_PATHS (macOS + Linux paths)
  3. None — caller falls back to the downloader (Task 3b)

Env + probe-paths are function parameters, not globals. Tests pass their own fixtures; production reads env inline. Keeps it deterministic.

Executability check

Unix: metadata.permissions().mode() & 0o111 != 0 against the resolved file. Non-unix: any existing file passes (Windows is explicitly out of Phase 1 per design doc non-goals; WSL workaround).

Tests (8 passing, strict TDD)

  • Env override used when executable
  • Env override skipped when: missing path, not-executable file (Unix), directory
  • System probe returns first hit in the list
  • Env override takes precedence over probe paths
  • Returns None when nothing found
  • SYSTEM_CHROME_PATHS covers both macOS (Google Chrome.app) and Linux (/usr/bin/google-chrome, /usr/bin/chromium)

Acceptance criteria satisfied (from design doc Task 3)

  • $VERTZ_CHROME_PATH takes precedence
  • System Chrome detected on macOS (/Applications/Google Chrome.app/...)
  • System Chrome detected on Linux (/usr/bin/google-chrome, /usr/bin/chromium)

Remaining for Task 3b (follow-up PR)

  • Chrome for Testing JSON index parsing + fixture
  • Download + SHA-256 verify
  • Cache manifest at ~/.vertz/chromium/current.json
  • $XDG_CACHE_HOME$TMPDIR fallback for read-only $HOME
  • macOS quarantine removal via xattr subprocess
  • wiremock dep for HTTP mocking

Test plan

  • cargo test -p vtz --lib server::screenshot — 26 passed (18 artifacts + 8 fetcher)
  • cargo fmt --all -- --check — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • Full pre-push hooks (including rust-clippy --all-targets, rust-test, TS build-typecheck/test, trojan-source) — all green 🎉

Relates to #2865. Follow-up for Task 3b to be opened after merge.

Task 3a of Phase 1. Ships the network-free half of the Chrome
binary resolver so Task 4 (pool) can start depending on a stable
`resolve_local_chrome` API. The download + cache-manifest half
is Task 3b (needs wiremock dep).

Public API:
- resolve_local_chrome(env_path, probe_paths) -> Option<PathBuf>
- SYSTEM_CHROME_PATHS: &[&str]  (macOS + Linux defaults)

Resolution order:
1. $VERTZ_CHROME_PATH (passed by the caller) if executable
2. First executable match in SYSTEM_CHROME_PATHS
3. None — caller falls back to downloader (landed in 3b)

Env + probe-paths are function parameters (not globals) to keep
tests deterministic. Production call site reads env inline.

Executability check: Unix permissions 0o111 bits against the
file's metadata. Non-unix: any existing file counts (Windows
isn't in Phase 1 scope per the design doc).

8 unit tests (strict TDD):
- env override used when executable
- env override skipped when missing / not-executable / directory
- system probe returns first hit
- env override takes precedence over probe paths
- returns None when nothing found
- SYSTEM_CHROME_PATHS covers macOS + Linux binaries

Acceptance criteria this PR satisfies (from design doc Task 3):
- [x] $VERTZ_CHROME_PATH takes precedence
- [x] System Chrome detected on macOS (/Applications/Google Chrome.app/...)
      and Linux (/usr/bin/google-chrome, /usr/bin/chromium)

Remaining for Task 3b:
- Chrome for Testing JSON resolution + fixture
- Download + SHA-256 verify
- Cache manifest at ~/.vertz/chromium/current.json
- $XDG_CACHE_HOME → $TMPDIR fallback for read-only $HOME
- macOS quarantine removal
- wiremock mocking

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@viniciusdacal viniciusdacal merged commit 3171a00 into main Apr 20, 2026
6 checks passed
matheuspoleza added a commit that referenced this pull request Apr 21, 2026
* feat(vtz): screenshot::fetcher download path — Task 3b [#2865]

Completes the Chrome binary resolver started in #2874 (Task 3a) with the
network half so Task 4 (pool) has a full resolver to depend on.

New public API in server::screenshot::fetcher:

  Platform::detect(os, arch) -> Result<Platform, PlatformError>
  Platform::cft_id(self) -> &'static str

  parse_versions(json) -> Result<ChromeVersions, VersionsError>
  ChromeVersions::url_for(platform) -> Result<&str, VersionsError>

  resolve_cache_dir(home, xdg_cache, tmp) -> Result<PathBuf, CacheDirError>

  verify_sha256(path, expected_hex) -> Result<(), VerifyError>
  unpack_chrome_zip(zip_path, dest_dir) -> Result<PathBuf, UnpackError>
  download_to_file(url, dest) -> Future<Output = Result<(), DownloadError>>
  remove_quarantine(path)  // best-effort, no-op on non-macOS

  CacheManifest { revision, binary_path, downloaded_at_epoch_secs }
  manifest_path(cache_dir), write_manifest(dir, m), read_manifest(dir)

  EnsureOptions { env_chrome_path, probe_paths, cache_dir,
                  versions_url, expected_revision, expected_sha256, platform }
  ensure_chrome(&opts) -> Future<Output = Result<PathBuf, EnsureError>>

ensure_chrome() orchestrates the full lifecycle:
  1. Local probe (env + system paths) — same fast path as 3a
  2. Cache manifest (if revision matches AND binary still executable)
  3. Fetch CfT versions JSON, pick platform URL, download to
     <cache_dir>/<rev>/chrome-headless-shell.zip, SHA-256 verify,
     unpack, strip macOS quarantine, write current.json, delete zip

Security / safety:
  - Zip extraction uses `enclosed_name()` to reject path traversal
  - SHA-256 compared case-insensitively via `sha2` (already in deps)
  - Cache dir falls back HOME → XDG_CACHE_HOME → TMPDIR via
    touch-probe, so read-only $HOME on CI doesn't break it
  - No $HOME/$XDG_CACHE_HOME/$TMPDIR reads in the library — callers
    pass them in, keeping tests deterministic

Dependency additions (native/vtz/Cargo.toml):
  - zip = { version = "2", default-features = false, features = ["deflate"] }
  - wiremock = "0.6" (dev-dependency only)

Tests (27 new, all in-process — no live network, no real Chrome):
  - Platform detection (linux/x86_64, macos/aarch64, macos/x86_64, rejects windows + linux/aarch64)
  - JSON index parsing: happy path + each error variant
  - Cache dir resolver: HOME / XDG / TMPDIR fallback + no-writable
  - SHA-256: match / mismatch / case-insensitive / missing file
  - Zip unpack: happy path / exec-bit preserved / binary missing / invalid zip / path traversal rejected
  - Download: stream to disk / non-2xx / connection refused / nested dest
  - Quarantine: non-fatal on missing path + on real file (macOS)
  - Manifest: roundtrip / missing / corrupt / no tmp leaks / creates dir
  - ensure_chrome: local-probe short-circuits / cached short-circuits /
    fresh download via wiremock / SHA mismatch / stale revision re-downloads

Remaining for Task 3: nothing. Next phase is Task 4 (pool).

Acceptance criteria met (from plans/2865-phase-1-headless-screenshot.md Task 3):
  - [x] \$VERTZ_CHROME_PATH takes precedence (from 3a)
  - [x] System Chrome detected on macOS + Linux (from 3a)
  - [x] Download resolves revision from fixture JSON; live CfT calls are not
        exercised in unit tests (no #[ignore] live-network tests included —
        gating those is deferred to Task 8's E2E suite)
  - [x] SHA-256 mismatch returns verify error (wraps into EnsureError::Verify)
  - [x] Second invocation skips download, reads from current.json
  - [x] wiremock crate used for HTTP mocking in unit tests
  - [x] Read-only \$HOME fallback exercised in a temp-dir test

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(vtz): address adversarial review findings for Task 3b [#2865]

Resolves 4 blockers + 4 should-fixes from the staff-reviewer pass.

B1. download_to_file now actually streams. Previously used .bytes() which
    buffered the whole body (100 MB+) in memory despite claiming to stream.
    Switched to bytes_stream() writing 8 KiB chunks, plus a MAX_DOWNLOAD_BYTES
    cap (500 MB) checked against Content-Length up front AND against the
    running total. New DownloadError::SizeCap variant. Partial file is
    removed on cap abort. Tests: content_length_over_cap +
    stream_exceeds_cap (both assert no file leak).

B2. ensure_chrome validates expected_sha256 is 64 hex chars before any
    other work. An empty or malformed SHA previously surfaced as a
    confusing mismatch message; worse, a future caller accidentally
    passing "" would effectively disable integrity enforcement. New
    EnsureError::InvalidExpectedSha. Test covers empty / short / long /
    non-hex inputs.

B3. unpack_chrome_zip rejects entries with S_IFLNK file-type bits. zip
    crate's add_symlink now trips UnsafePath. Also masks permission bits
    when applying mode to extracted files (0o7777 instead of the raw mode)
    so file-type bits from the archive can't bleed into the local file.

B4 + S1. File lock around download/verify/unpack via fs2::try_lock_exclusive
    on <cache_dir>/.lock. Concurrent vtz processes on the same cache hit
    EnsureError::CacheBusy instead of racing on the zip file and manifest.
    After acquiring the lock, we re-read the manifest in case a sibling
    finished while we were waiting. Scope guard (CleanupOnDrop) removes
    the rev dir on error so retries start clean. Tests: cache_busy on
    held lock + clean_up_rev_dir on SHA mismatch.

S2. remove_quarantine uses absolute /usr/bin/xattr (defeats PATH injection
    on macOS). xattr ships with macOS base so the absolute path is stable.

S4. write_manifest uses explicit cache_dir.join("current.json.tmp")
    instead of the fragile with_extension("json.tmp") trick.

S5. ensure_chrome checks the versions-index HTTP status before calling
    .text(). Previously a 404/500 would return garbage body to parse_versions
    which would fail with a confusing parse error. New
    EnsureError::VersionsHttpStatus variant with test.

S7. Test ensure_chrome_returns_cached_when_manifest_matches now uses a
    DUMMY_SHA256 constant (valid format, wrong content) plus an unreachable
    versions URL, so a regression in the cache short-circuit would fail
    on the network or verify step rather than silently passing.

S9. Tightened visibility: ChromeVersions, PlatformDownload, parse_versions,
    CacheManifest + fields, manifest_path, write_manifest, read_manifest,
    now_epoch_secs, verify_sha256, unpack_chrome_zip, download_to_file,
    remove_quarantine are all pub(crate) now. The public API surface is
    just the entry points (resolve_local_chrome, ensure_chrome, Platform,
    resolve_cache_dir, SYSTEM_CHROME_PATHS) + the error enums needed for
    pattern-matching from callers.

Test count: 53 -> 78 fetcher tests. Full workspace: 5154 passed, 0 failed.

Not addressed in this follow-up (deferred with rationale):
- S3 (TOCTOU on is_executable_file): caller contract — cache_dir must be
  owned by the running user. Worth a doc comment but not a code change.
- S6 (absolute binary_path in manifest): breaks portability if user
  moves cache. Valid concern but larger refactor; Task 4 can decide
  whether relative paths fit the pool's needs.
- S8 (in-process concurrent ensure_chrome test): covered in spirit by
  the cache_busy test. A true in-process race would need Pool (Task 4)
  anyway.
- N1 (binary-name anchoring): CfT zips have a single chrome-headless-shell
  entry; last-wins is fine until that changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(vtz): distinguish cache lock contention from infra io errors [#2865]

Re-review SF1: `try_lock_exclusive` returns Err for both "lock held"
(WouldBlock) and real I/O failures (disk full, permission denied,
EBADF). Mapping both to CacheBusy would mislead users into retrying
what's actually a system problem.

Now:
  WouldBlock  -> EnsureError::CacheBusy
  other Err   -> EnsureError::Io(e)

Also added a comment near the lock handling documenting that flock is
released by the kernel on fd close, so kill/panic safety doesn't need
a Drop impl — the `std::fs::File` drop that always runs on unwind is
sufficient. Prevents a future well-meaning refactor from adding a
redundant (and potentially racy) custom guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matheuspoleza added a commit that referenced this pull request Apr 22, 2026
…#2935)

Phase 1 landed across #2872, #2873, #2874, #2910, #2913, #2915, #2916,
#2918, #2919 without adding a per-PR changeset. This one consolidated
changeset captures the full feature — vertz_browser_screenshot MCP
tool + artifact route + template + mint-docs — so the release PR picks
it up in the next @vertz/runtime + @vertz/create-vertz-app bump.

Fixed-version group means the bump cascades to every @vertz/* package;
patch is correct (new feature but pre-v1 per policies.md).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matheuspoleza added a commit that referenced this pull request Apr 22, 2026
Per .claude/rules/local-phase-workflow.md section 6 "Wiki Archival":
completed phase plans move from plans/ to plans/archived/ (wiki fallback
because vertz.wiki repo doesn't exist yet).

Archived (Phase 1 is done — merged across #2871, #2872, #2873, #2874,
#2910, #2913, #2915, #2916, #2918, #2919; published as
@vertz/runtime@0.2.78 + @vertz/create-vertz-app@0.2.78):

  plans/2865-chromium-poc-research.md
  plans/2865-chromium-poc-results.md
  plans/2865-phase-1-binary-size.md
  plans/2865-phase-1-headless-screenshot.md

Kept active: plans/2865-agent-visual-handoff.md — vision/roadmap doc
still covers Phases 2 (impersonation), 3 (overlay), 4 (current-tab),
5 (template rollout). Those phases remain blocked on @vertz/auth
consolidation and compiler dev/prod mode; the roadmap stays in plans/
until each phase ships.

The Phase 1 retrospective stays in plans/post-implementation-reviews/
per the same rule — reviews live at their own path, not under archived/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matheuspoleza added a commit that referenced this pull request Apr 22, 2026
Per .claude/rules/local-phase-workflow.md section 6 "Wiki Archival":
completed phase plans move from plans/ to plans/archived/ (wiki fallback
because vertz.wiki repo doesn't exist yet).

Archived (Phase 1 is done — merged across #2871, #2872, #2873, #2874,
#2910, #2913, #2915, #2916, #2918, #2919; published as
@vertz/runtime@0.2.78 + @vertz/create-vertz-app@0.2.78):

  plans/2865-chromium-poc-research.md
  plans/2865-chromium-poc-results.md
  plans/2865-phase-1-binary-size.md
  plans/2865-phase-1-headless-screenshot.md

Kept active: plans/2865-agent-visual-handoff.md — vision/roadmap doc
still covers Phases 2 (impersonation), 3 (overlay), 4 (current-tab),
5 (template rollout). Those phases remain blocked on @vertz/auth
consolidation and compiler dev/prod mode; the roadmap stays in plans/
until each phase ships.

The Phase 1 retrospective stays in plans/post-implementation-reviews/
per the same rule — reviews live at their own path, not under archived/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matheuspoleza added a commit that referenced this pull request Apr 22, 2026
* docs: retrospective for #2865 Phase 1 (headless screenshot)

Retrospective per Definition of Done in
.claude/rules/design-and-planning.md. Captures what went well (10
blockers caught by adversarial review, Arc::strong_count guard as the
minimal race-free fix, RwLock refactor as the right architecture),
what went wrong (missed changesets on all 8 PRs, unrelated `'vtz':`
changeset bug blocking the release pipeline, dead enum variants as a
contract lie), lessons (CI check for changeset packages, FakeHandle
should mirror production concurrency shape, preview APIs should ship
with emitters or stay out), and the open follow-up backlog.

Also documents the active P5 dogfooding window (2026-04-22 to
2026-05-06) — the next 3 PRs touching .tsx in examples/linear/ should
include a vertz_browser_screenshot artifact link.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(plans): archive #2865 Phase 1 docs (Phase 1 shipped as 0.2.78)

Per .claude/rules/local-phase-workflow.md section 6 "Wiki Archival":
completed phase plans move from plans/ to plans/archived/ (wiki fallback
because vertz.wiki repo doesn't exist yet).

Archived (Phase 1 is done — merged across #2871, #2872, #2873, #2874,
#2910, #2913, #2915, #2916, #2918, #2919; published as
@vertz/runtime@0.2.78 + @vertz/create-vertz-app@0.2.78):

  plans/2865-chromium-poc-research.md
  plans/2865-chromium-poc-results.md
  plans/2865-phase-1-binary-size.md
  plans/2865-phase-1-headless-screenshot.md

Kept active: plans/2865-agent-visual-handoff.md — vision/roadmap doc
still covers Phases 2 (impersonation), 3 (overlay), 4 (current-tab),
5 (template rollout). Those phases remain blocked on @vertz/auth
consolidation and compiler dev/prod mode; the roadmap stays in plans/
until each phase ships.

The Phase 1 retrospective stays in plans/post-implementation-reviews/
per the same rule — reviews live at their own path, not under archived/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants