Skip to content

fix: ungate server resource staging#1500

Merged
Nikhil (shadowfax92) merged 2 commits into
mainfrom
fix/copy-claw-server-resources
Jul 1, 2026
Merged

fix: ungate server resource staging#1500
Nikhil (shadowfax92) merged 2 commits into
mainfrom
fix/copy-claw-server-resources

Conversation

@shadowfax92

Copy link
Copy Markdown
Contributor

Summary

  • stage BrowserOS and BrowserClaw server resource downloads for every product while retaining OS/arch filters
  • copy both server resource trees for every product so Chromium resource bundle targets can consume paths selected by GN flags
  • update real-config tests for download/copy filtering across products and target platforms

Test plan

  • cd packages/browseros && uv run python -m unittest bos_build.steps.resources.resources_test bos_build.steps.storage.download_test
  • cd packages/browseros && uv run python -m unittest discover -s bos_build -t . -p "*_test.py"

Notes

  • Adversarial review verdict: clean.
  • Did not run autoninja because this patch does not change Chromium source code.

@shadowfax92

Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

@shadowfax92

Copy link
Copy Markdown
Contributor Author

Claude (@claude) review

@github-actions github-actions Bot added the fix label Jul 1, 2026
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished Nikhil (@shadowfax92)'s task in 1m 34s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing changes
  • Posting review

Overall: LGTM. The change is clean, minimal, and correctly implements the stated goal.

What this PR does

Removes product guards from all server and claw-server resource operations in both download_resources.yaml and copy_resources.yaml. Because _product_matches(None, product_id) unconditionally returns True (download.py:344), omitting the field is the correct idiom for "run for all products." OS/arch filtering is preserved exactly.

File-by-file notes

config/download_resources.yaml / config/copy_resources.yaml

Both files previously used product: ["browseros"] and product: ["browserclaw"] to gate the respective server trees. Removing those fields is the right mechanism — the _product_matches helper already documents this convention: product: all is explicitly rejected in favour of a missing field (download.py:347). The change is symmetric across all 10 operations (5 per tree × 2 files).

steps/resources/resources_test.py

  • Renamed tests accurately reflect the expanded scope — both server resource trees are now asserted for both products.
  • The previously split tests (browseros_product → browseros server only, browserclaw_product → claw only) are correctly consolidated into tests that verify both trees land regardless of product. The assertFalse calls on the "other" server tree are correctly replaced with assertEqual assertions.

steps/storage/download_test.py

  • test_real_config_includes_server_artifacts_by_target now verifies both browseros_server and browseros_claw_server entries per platform/arch pair. The if "Server Resources" in op["name"] guard correctly isolates server entries in case other download ops are added later.
  • test_real_config_includes_both_macos_arches_for_universal now expects all 4 entries (2 server × 2 arch) — correct for a universal build.
  • test_real_config_includes_server_artifacts_for_browserclaw_product correctly adds the browseros_server tuple to the expected results for browserclaw, matching the new ungated behavior.

Minor observations (no action needed)

  1. _filter_operations in download.py still handles product filtering (lines 333–335). With no operations in the current YAML carrying a product field, that branch is never exercised — but it's not dead code to remove; it's a still-valid capability for future entries.

  2. download_resources.yaml currently holds only server resource operations. If non-server ops are ever added, the if "Server Resources" in op["name"] guard in the tests will silently exclude them from these assertions — worth keeping in mind, but not a problem today.

Test plan verification

The PR test commands exercise both the resource-copy and the download-filter paths:

uv run python -m unittest bos_build.steps.resources.resources_test bos_build.steps.storage.download_test

Coverage looks complete for the changed behaviour.


@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes product-specific gates (product: ["browseros"] / product: ["browserclaw"]) from both the download and copy resource configs, so that both the BrowserOS server and BrowserClaw server resource trees are staged for every product build. OS/arch filters are preserved unchanged.

  • download_resources.yaml / copy_resources.yaml: 10 product filter lines removed (5 for browseros_server, 5 for browseros_claw_server) so both artifact sets are downloaded and copied regardless of which product is being built.
  • Tests updated: resources_test.py and download_test.py flip previously-negative assertions (assertFalse) to positive ones and add the formerly-missing artifact entries, correctly reflecting the new "both trees always present" invariant.

Confidence Score: 5/5

Safe to merge — the change removes product gates from YAML configs and the corresponding test assertions are correctly updated to match.

All four files change in a consistent, mechanical way: product filter lines are removed from download and copy configs, and the tests that previously expected those resources to be absent for the wrong product now correctly assert they are present. The OS/arch filters that actually control which binary is fetched for a given host are untouched, so no spurious downloads or overwrites can occur at runtime.

No files require special attention.

Important Files Changed

Filename Overview
packages/browseros/bos_build/config/download_resources.yaml Removes product filters from all 10 server resource download entries; OS/arch gates are intact. Change is intentional and matches the PR description.
packages/browseros/bos_build/config/copy_resources.yaml Removes product filters from all 10 copy operations in parallel with the download config. No logic errors; OS/arch filters preserved.
packages/browseros/bos_build/steps/resources/resources_test.py Tests correctly updated: previously-false assertions for the other product's destination flipped to positive equality checks, and test names clarified to reflect product-agnostic behavior.
packages/browseros/bos_build/steps/storage/download_test.py Download config tests updated across all five platforms to expect both server artifact sets for the default browseros product, and the browserclaw product test now includes the BrowserOS server entries as well.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Build triggered for any product\nbrowseros / browserclaw] --> B{OS/Arch Filter}
    B -->|macos arm64| C1[Download browseros_server darwin-arm64]
    B -->|macos arm64| C2[Download browseros_claw_server darwin-arm64]
    B -->|macos x64| D1[Download browseros_server darwin-x64]
    B -->|macos x64| D2[Download browseros_claw_server darwin-x64]
    B -->|linux arm64| E1[Download browseros_server linux-arm64]
    B -->|linux arm64| E2[Download browseros_claw_server linux-arm64]
    B -->|linux x64| F1[Download browseros_server linux-x64]
    B -->|linux x64| F2[Download browseros_claw_server linux-x64]
    B -->|windows x64| G1[Download browseros_server windows-x64]
    B -->|windows x64| G2[Download browseros_claw_server windows-x64]
    C1 & C2 --> H[Copy to chrome/browser/browseros/server/resources & chrome/browser/browseros/claw_server/resources]
    H --> I[Chromium GN flags select which tree to use at build time]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Build triggered for any product\nbrowseros / browserclaw] --> B{OS/Arch Filter}
    B -->|macos arm64| C1[Download browseros_server darwin-arm64]
    B -->|macos arm64| C2[Download browseros_claw_server darwin-arm64]
    B -->|macos x64| D1[Download browseros_server darwin-x64]
    B -->|macos x64| D2[Download browseros_claw_server darwin-x64]
    B -->|linux arm64| E1[Download browseros_server linux-arm64]
    B -->|linux arm64| E2[Download browseros_claw_server linux-arm64]
    B -->|linux x64| F1[Download browseros_server linux-x64]
    B -->|linux x64| F2[Download browseros_claw_server linux-x64]
    B -->|windows x64| G1[Download browseros_server windows-x64]
    B -->|windows x64| G2[Download browseros_claw_server windows-x64]
    C1 & C2 --> H[Copy to chrome/browser/browseros/server/resources & chrome/browser/browseros/claw_server/resources]
    H --> I[Chromium GN flags select which tree to use at build time]
Loading

Reviews (1): Last reviewed commit: "fix: ungate server resource staging" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes product-level gates (product: ["browseros"] / product: ["browserclaw"]) from all server resource download and copy operations, so both the BrowserOS and BrowserClaw server resource trees are staged for every build product while OS/arch filters remain in place. Tests are updated accordingly to assert that both resource trees land in Chromium's source tree regardless of the active product.

  • download_resources.yaml / copy_resources.yaml: Dropped the product field from all 10 server resource entries in each config (5 BrowserOS + 5 BrowserClaw across all OS/arch targets), making both sets unconditionally staged per platform.
  • resources_test.py / download_test.py: Renamed and revised the real-config integration tests to verify both server trees are downloaded and copied for both browseros and browserclaw products; the universal macOS download test now correctly expects 4 entries (ARM64 + x64 for each server).

Confidence Score: 5/5

Safe to merge — the change is a straightforward removal of product gates from YAML config entries with matching test updates that fully cover the new behavior.

The diff is mechanical and self-consistent: 10 product: lines removed from each config, OS/arch filters left intact, and every affected test rewritten to match. The universal macOS download test is correctly expanded to 4 expected entries. No logic changes in the implementation code, no new code paths introduced.

No files require special attention.

Important Files Changed

Filename Overview
packages/browseros/bos_build/config/download_resources.yaml Removed product filter from all 10 server resource download operations (5 BrowserOS + 5 BrowserClaw across all OS/arch combos), so both are downloaded for every build product; OS/arch filters remain intact.
packages/browseros/bos_build/config/copy_resources.yaml Removed product filter from all 10 server resource copy operations (5 BrowserOS + 5 BrowserClaw across all OS/arch combos), mirroring the download config change; OS/arch filters remain intact.
packages/browseros/bos_build/steps/resources/resources_test.py Updated copy-resource tests to assert that both server trees are copied for both browseros and browserclaw products; previously only the matching server tree was expected per product.
packages/browseros/bos_build/steps/storage/download_test.py Updated download config tests to expect both BrowserOS and BrowserClaw server entries for every OS/arch target and for the browserclaw product; universal macOS test now correctly expects 4 entries instead of 2.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Build Triggered] --> B{OS Filter}
    B -->|macOS ARM64| C[Download/Copy BrowserOS Server – darwin-arm64]
    B -->|macOS ARM64| D[Download/Copy BrowserClaw Server – darwin-arm64]
    B -->|macOS x64| E[Download/Copy BrowserOS Server – darwin-x64]
    B -->|macOS x64| F[Download/Copy BrowserClaw Server – darwin-x64]
    B -->|Linux ARM64| G[Download/Copy BrowserOS Server – linux-arm64]
    B -->|Linux ARM64| H[Download/Copy BrowserClaw Server – linux-arm64]
    B -->|Linux x64| I[Download/Copy BrowserOS Server – linux-x64]
    B -->|Linux x64| J[Download/Copy BrowserClaw Server – linux-x64]
    B -->|Windows x64| K[Download/Copy BrowserOS Server – windows-x64]
    B -->|Windows x64| L[Download/Copy BrowserClaw Server – windows-x64]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Build Triggered] --> B{OS Filter}
    B -->|macOS ARM64| C[Download/Copy BrowserOS Server – darwin-arm64]
    B -->|macOS ARM64| D[Download/Copy BrowserClaw Server – darwin-arm64]
    B -->|macOS x64| E[Download/Copy BrowserOS Server – darwin-x64]
    B -->|macOS x64| F[Download/Copy BrowserClaw Server – darwin-x64]
    B -->|Linux ARM64| G[Download/Copy BrowserOS Server – linux-arm64]
    B -->|Linux ARM64| H[Download/Copy BrowserClaw Server – linux-arm64]
    B -->|Linux x64| I[Download/Copy BrowserOS Server – linux-x64]
    B -->|Linux x64| J[Download/Copy BrowserClaw Server – linux-x64]
    B -->|Windows x64| K[Download/Copy BrowserOS Server – windows-x64]
    B -->|Windows x64| L[Download/Copy BrowserClaw Server – windows-x64]
Loading

Reviews (2): Last reviewed commit: "fix: ungate server resource staging" | Re-trigger Greptile

@shadowfax92 Nikhil (shadowfax92) merged commit 355f8a8 into main Jul 1, 2026
4 checks passed
@shadowfax92 Nikhil (shadowfax92) deleted the fix/copy-claw-server-resources branch July 1, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant