Skip to content

chore(browseros): restructure bos_build into toolset layout#1503

Merged
Nikhil (shadowfax92) merged 5 commits into
mainfrom
chore/bos-build-toolset-layout
Jul 2, 2026
Merged

chore(browseros): restructure bos_build into toolset layout#1503
Nikhil (shadowfax92) merged 5 commits into
mainfrom
chore/bos-build-toolset-layout

Conversation

@shadowfax92

Copy link
Copy Markdown
Contributor

Summary

  • Re-house packages/browseros/bos_build/ so the top level reads as three toolsets: BUILD (steps/ on the core/ engine), RELEASE (release/ + release/ota/), DEV (patchkit/) — over shared plumbing (lib/) and product data (products/).
  • Move-only: git mv + import rewrites + docstring/README path updates. No behavior change; history follows every file (git log --follow).
  • Lands ahead of the feed-publisher and extension-release work so those are born in the right place.

Moves

  • core/lib/: env, utils, logger, paths, notify, sparkle, versions (+test), testing (+test) — core/ now holds only the engine (step registry, planner, runner, pipeline, resolver, context, events, product descriptor model)
  • core/server_binaries*products/ (it is product data); steps/storage/r2.pylib/r2.py (shared client used by release/ota/storage)
  • steps/release/release/ (flattened); steps/ota/release/ota/; steps/ keeps only @step pipeline steps
  • core/__init__.py exports engine names only; EnvConfig/Notifier/get_notifier moved to lib/__init__.py; steps/storage/__init__.py slimmed to step re-exports

Design

Staged git mv in dependency order (plumbing → data/client → toolsets → docs), one green commit each, so rename detection stays clean and every intermediate state passes the full harness. Step registry contract untouched: steps/__init__.py import order = registration order, loaded via import_module("bos_build.steps"); release/ota modules were never registered steps. Lazy cycle-dodging imports keep their laziness (notify→events, env/logger→paths, server_binaries→products registry).

Test plan

  • uv run python -m unittest discover -s bos_build -t . -p "*_test.py" → 352 tests OK after every commit
  • uv run ruff check bos_build clean after every commit
  • CLI smokes after every commit: browseros build --list, product doctor, release --help, ota --help, dev extract --help
  • Old-path sweep (dotted + slash forms, incl. mock.patch strings, workflows, docs): zero hits
  • Adversarial context-free review: clean apart from one stale docstring path, fixed

🤖 Generated with Claude 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 chore label Jul 2, 2026
@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown

Claude finished Nikhil (@shadowfax92)'s task in 2m 38s —— View job


PR Review: chore(browseros): restructure bos_build into toolset layout

  • Read CLAUDE.md for repo guidelines
  • Examine the diff and changed files
  • Review lib/__init__.py (new file)
  • Review core/__init__.py (modified exports)
  • Review steps/storage/__init__.py (slimmed)
  • Review README changes
  • Check for stale import paths or broken references
  • Post review findings

Overall

This is a clean, well-sequenced move-only refactor. The 5-commit order (plumbing → data/client → toolsets → docs) is correct — each builds on the last with no intermediate broken states. The intent is clear and the execution matches it.

What's right

Import path sweep — zero stale references found. Searched for all old dotted paths (bos_build.core.env, bos_build.core.utils, bos_build.core.notify, bos_build.steps.ota, bos_build.steps.release, bos_build.steps.storage.r2) across all .py, .yml, and workflow files — zero hits.

core/__init__.py correctly stripped to engine-only (Context, ArtifactRegistry, Step, ValidationError). EnvConfig/Notifier/get_notifier correctly migrated to lib/__init__.py.

steps/storage/__init__.py correctly slimmed — R2 utilities are no longer leaked through this __init__; callers now import from lib.r2 directly. The slimmed __all__ matches the new intent.

Lazy cycle-dodging imports preserved correctly:

  • notify.pycore.events (inside slack_subscriber) — still lazy ✓
  • products/server_binaries.pyproducts/__init__.SERVER_BUNDLES — lazy from . import SERVER_BUNDLES avoids the type-definition cycle ✓

release/ota/common.py: The from ...lib.sparkle import sparkle_sign_file as sparkle_sign_file explicit re-export alias is the right pattern — as name makes it a public re-export that ruff accepts as intentional.

README accurately reflects the new layout.


Minor observations (pre-existing, not introduced here)

Two functions in release/common.py are product-hardcoded. Since the PR description explicitly says feed-publisher and extension-release work will land here next, flagging these as the natural follow-up:

generate_appcast_item (release/common.py:163):

<title>BrowserOS - {version}</title><link>https://browseros.com</link>

Title and link are hardcoded to the browseros product. If browserclaw ever goes through this path these will be wrong.

generate_release_notes (release/common.py:188):

notes = f"""## BrowserOS v{version}

Same issue. Both functions already receive enough context to accept a ProductDescriptor argument — easy to fix when the multi-product release work lands.

Fix these when multi-product release lands →


Verdict

Approved. Clean move-only PR with no behavior change, comprehensive import path cleanup, and correct cycle avoidance. The two hardcoded product name issues in release/common.py pre-date this PR and are the right thing to fix in the incoming release/publish work rather than here.

@shadowfax92 Nikhil (shadowfax92) merged commit f53f19c into main Jul 2, 2026
7 checks passed
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

A large-scale move-only restructure of packages/browseros/bos_build/ that separates the codebase into three toolsets — BUILD (steps/ on the core/ engine), RELEASE (release/ + release/ota/), and DEV (patchkit/) — with shared plumbing extracted to lib/ and product data moved to products/. All 87 changed files are import rewrites, __init__.py re-exports, and docstring path corrections; no behavioral changes are introduced.

  • core/ shared helpers (env, utils, logger, paths, notify, sparkle, versions, testing) migrate to lib/, and core/server_binaries migrates to products/, leaving core/ as a pure engine.
  • steps/release/ and steps/ota/ are promoted to top-level release/ and release/ota/, each with a new eager-importing __init__.py that exposes an AVAILABLE_MODULES registry and re-exports the public API.

Confidence Score: 4/5

Move-only refactor with import rewrites; no logic changes. The one non-cosmetic issue is a stale path in an operator-facing error message in sign_binary.py.

All 87 files are import rewrites, init.py restructures, or docstring corrections. Every changed import was verified to point at the correct new location. The only slip is a developer-facing error string in sign_server_bundle_macos that still references build/common/server_binaries.py, a path that never existed — it should say bos_build/products/server_binaries.py. This would misdirect an engineer trying to register a newly added binary. Everything else is clean.

packages/browseros/bos_build/release/ota/sign_binary.py — the stale error-message path in sign_server_bundle_macos.

Important Files Changed

Filename Overview
packages/browseros/bos_build/release/ota/sign_binary.py Renamed from steps/ota/sign_binary.py; imports updated to lib/ and products/. One stale error-message path (build/common/server_binaries.py → bos_build/products/server_binaries.py) slipped through the old-path sweep.
packages/browseros/bos_build/lib/init.py New file establishing the lib/ package public API; exports EnvConfig, Notifier, and get_notifier moved from core/init.py. Clean and minimal.
packages/browseros/bos_build/core/init.py Trimmed to engine-only exports (Context, ArtifactRegistry, Step, ValidationError); EnvConfig, Notifier, get_notifier correctly removed and re-homed in lib/init.py.
packages/browseros/bos_build/release/init.py New package init that eagerly imports all five release modules and exposes AVAILABLE_MODULES registry; mirrors the prior steps/release/init.py pattern exactly.
packages/browseros/bos_build/release/ota/init.py New OTA sub-package init re-exporting common utilities, sign_binary helpers, and ServerOTAModule; comprehensive all covers all public symbols from ota/common.py.
packages/browseros/bos_build/products/server_binaries.py Moved from core/server_binaries.py; types and lazy registry lookups unchanged, now properly co-located with the product definitions it serves.
packages/browseros/bos_build/steps/storage/init.py Correctly slimmed to step re-exports only (UploadModule, DownloadResourcesModule); R2 client utilities now live in lib/r2.py and are no longer surfaced through this package.
packages/browseros/bos_build/cli/ota.py Import paths updated from core.* and steps.ota/* to lib.* and release.ota/*; all six changed imports are correct.
packages/browseros/bos_build/release/ota/server.py Renamed from steps/ota/server.py; all imports updated correctly; cross-package reference to steps/storage/download.extract_artifact_zip is intentional and resolves correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    cli["cli/\nbuild · source · product\ndev · release · ota"]

    subgraph core ["core/ (engine)"]
        ctx["context"]
        step["step registry"]
        planner["planner"]
        runner["runner"]
        pipeline["pipeline"]
        resolver["resolver"]
    end

    subgraph lib ["lib/ (plumbing)"]
        env["env"]
        utils["utils"]
        notify["notify"]
        paths["paths"]
        sparkle["sparkle"]
        versions["versions"]
        r2["r2"]
        testing["testing"]
    end

    subgraph products ["products/"]
        sbins["server_binaries"]
        proddefs["browseros/ · browserclaw/"]
        doctor["doctor"]
    end

    subgraph steps ["steps/ (BUILD toolset)"]
        setup["setup"]
        resources["resources"]
        patches["patches"]
        compile["compile"]
        sign["sign"]
        package["package"]
        storage["storage"]
    end

    subgraph release ["release/ (RELEASE toolset)"]
        rcommon["common"]
        rlist["list"]
        rappcast["appcast"]
        rgithub["github"]
        rpublish["publish"]
        rdownload["download"]
        subgraph ota ["ota/"]
            otacommon["common"]
            otasign["sign_binary"]
            otaserver["server"]
        end
    end

    patchkit["patchkit/ (DEV toolset)"]

    cli --> core
    cli --> lib
    cli --> release
    core --> lib
    steps --> core
    steps --> lib
    steps --> products
    release --> core
    release --> lib
    release --> products
    ota --> lib
    ota --> products
    products --> core
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
    cli["cli/\nbuild · source · product\ndev · release · ota"]

    subgraph core ["core/ (engine)"]
        ctx["context"]
        step["step registry"]
        planner["planner"]
        runner["runner"]
        pipeline["pipeline"]
        resolver["resolver"]
    end

    subgraph lib ["lib/ (plumbing)"]
        env["env"]
        utils["utils"]
        notify["notify"]
        paths["paths"]
        sparkle["sparkle"]
        versions["versions"]
        r2["r2"]
        testing["testing"]
    end

    subgraph products ["products/"]
        sbins["server_binaries"]
        proddefs["browseros/ · browserclaw/"]
        doctor["doctor"]
    end

    subgraph steps ["steps/ (BUILD toolset)"]
        setup["setup"]
        resources["resources"]
        patches["patches"]
        compile["compile"]
        sign["sign"]
        package["package"]
        storage["storage"]
    end

    subgraph release ["release/ (RELEASE toolset)"]
        rcommon["common"]
        rlist["list"]
        rappcast["appcast"]
        rgithub["github"]
        rpublish["publish"]
        rdownload["download"]
        subgraph ota ["ota/"]
            otacommon["common"]
            otasign["sign_binary"]
            otaserver["server"]
        end
    end

    patchkit["patchkit/ (DEV toolset)"]

    cli --> core
    cli --> lib
    cli --> release
    core --> lib
    steps --> core
    steps --> lib
    steps --> products
    release --> core
    release --> lib
    release --> products
    ota --> lib
    ota --> products
    products --> core
Loading

Comments Outside Diff (1)

  1. packages/browseros/bos_build/release/ota/sign_binary.py, line 370-373 (link)

    P2 Stale path in the user-facing error message — build/common/server_binaries.py never existed. Before this PR the file lived at core/server_binaries.py; after this PR it is at bos_build/products/server_binaries.py. The error fires when a new binary is added to the server bundle without being registered, so the developer will immediately try to open the path it references.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/browseros/bos_build/release/ota/sign_binary.py
    Line: 370-373
    
    Comment:
    Stale path in the user-facing error message — `build/common/server_binaries.py` never existed. Before this PR the file lived at `core/server_binaries.py`; after this PR it is at `bos_build/products/server_binaries.py`. The error fires when a new binary is added to the server bundle without being registered, so the developer will immediately try to open the path it references.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/browseros/bos_build/release/ota/sign_binary.py:370-373
Stale path in the user-facing error message — `build/common/server_binaries.py` never existed. Before this PR the file lived at `core/server_binaries.py`; after this PR it is at `bos_build/products/server_binaries.py`. The error fires when a new binary is added to the server bundle without being registered, so the developer will immediately try to open the path it references.

```suggestion
        log_error(
            "Unknown executables found under resources/bin/ not registered in "
            "MACOS_SERVER_BINARIES (see bos_build/products/server_binaries.py):"
        )
```

Reviews (1): Last reviewed commit: "chore(browseros): fix stale versions pat..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restructures bos_build/ into three named toolsets — BUILD (steps/ on core/), RELEASE (release/), and DEV (patchkit/) — over shared plumbing (lib/) and product data (products/). It is a pure move-and-rewrite: no logic is changed, only file locations and import paths.

  • core/ is slimmed to the engine only (context, step registry, planner, runner, pipeline, resolver, events, product descriptor); the infrastructure files (env, utils, logger, paths, notify, sparkle, versions, testing) move to lib/
  • steps/release/ and steps/ota/ flatten to top-level release/ and release/ota/; core/server_binaries.py moves to products/; steps/storage/r2.py moves to lib/r2.py
  • All import rewrites are consistent and complete: no stale bos_build.core.utils/env/paths/notify/sparkle/testing/versions/server_binaries references remain

Confidence Score: 5/5

Pure structural reorganization with no logic changes; safe to merge.

All 87 changed files are import rewrites and file moves. The lazy-import cycle guards in notify.py and server_binaries.py were correctly preserved. No stale mock.patch strings, no dangling re-exports, and no behavioral changes were introduced. The new init.py files for lib/, release/, and release/ota/ accurately reflect what was previously spread across core/ and steps/.

No files require special attention. The package init.py files (lib, release, release/ota) are the most structurally significant changes, and all were verified to be consistent with the rest of the codebase.

Important Files Changed

Filename Overview
packages/browseros/bos_build/lib/init.py New package init re-exporting EnvConfig, Notifier, get_notifier from the newly promoted lib/ plumbing layer; mirrors what core/init.py previously exported for these symbols.
packages/browseros/bos_build/core/init.py Slimmed to engine-only exports (Context, ArtifactRegistry, Step, ValidationError); EnvConfig, Notifier, get_notifier removed and promoted to lib/init.py.
packages/browseros/bos_build/release/init.py Moved from steps/release/init.py with identical content; eagerly imports all five release module classes and re-exports common helpers.
packages/browseros/bos_build/release/ota/init.py Moved from steps/ota/init.py with identical content; exports OTA helpers, sign functions, and module registry; get_appcast_path intentionally omitted (accessed directly from ota.common by cli/ota.py).
packages/browseros/bos_build/steps/storage/init.py Stripped of R2 utility re-exports (moved to lib/r2.py); now correctly exports only the two step classes UploadModule and DownloadResourcesModule.
packages/browseros/bos_build/lib/r2.py Moved from steps/storage/r2.py; internal imports updated to use sibling lib/ files (.env, .utils) — both relative paths are correct for the new location.
packages/browseros/bos_build/products/server_binaries.py Moved from core/server_binaries.py; lazy registry access (from . import SERVER_BUNDLES) preserved to avoid the import cycle with product files that import these types.
packages/browseros/bos_build/lib/notify.py Moved from core/notify.py; lazy import of core.events inside slack_subscriber correctly preserved to avoid a notify→events circular import at module load time.
packages/browseros/bos_build/lib/testing.py Moved from core/testing.py; imports from ..core.products (correct, products.py stays in core/) and ..core.context remain valid after the restructuring.
packages/browseros/bos_build/cli/build.py All three lazy EnvConfig imports and top-level utility imports correctly redirected to lib/; no other changes.
packages/browseros/bos_build/cli/ota.py Imports updated: sparkle_sign_file from lib/sparkle, R2 helpers from lib/r2, OTA modules from release.ota — all paths correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["cli/ (build, ota, release, source, product, dev)"]

    subgraph CORE["core/ — Engine only"]
        CTX["context.py"]
        STEP["step.py"]
        PLAN["planner.py"]
        RUN["runner.py"]
        PIPE["pipeline.py"]
        RES["resolver.py"]
        EVENTS["events.py"]
        PROD["products.py"]
    end

    subgraph LIB["lib/ — Shared plumbing"]
        ENV["env.py"]
        UTILS["utils.py"]
        PATHS["paths.py"]
        NOTIFY["notify.py"]
        SPARKLE["sparkle.py"]
        VERS["versions.py"]
        R2["r2.py"]
        TEST["testing.py"]
    end

    subgraph PRODUCTS["products/ — Product data"]
        SB["server_binaries.py"]
        DOCTOR["doctor.py"]
        BOS["browseros/product.py"]
        BCLAW["browserclaw/product.py"]
    end

    subgraph BUILD["steps/ — BUILD toolset"]
        SETUP["setup/"]
        COMPILE["compile/"]
        SIGN["sign/"]
        PACKAGE["package/"]
        STORAGE["storage/"]
    end

    subgraph RELEASE["release/ — RELEASE toolset"]
        RCOMMON["common.py"]
        RPUB["publish.py"]
        RLIST["list.py"]
        subgraph OTA["ota/ — OTA subtoolset"]
            OTACOMMON["common.py"]
            OTASERVER["server.py"]
            OTASIGN["sign_binary.py"]
        end
    end

    subgraph DEV["patchkit/ — DEV toolset"]
        BATCH["batch_apply.py"]
        EXTRACT["extract/"]
    end

    CLI --> CORE
    CLI --> LIB
    CLI --> RELEASE
    CLI --> BUILD
    CORE --> LIB
    BUILD --> CORE
    BUILD --> LIB
    BUILD --> PRODUCTS
    RELEASE --> LIB
    RELEASE --> CORE
    DEV --> LIB
    DEV --> CORE
    PRODUCTS --> CORE
    OTA --> LIB
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
    CLI["cli/ (build, ota, release, source, product, dev)"]

    subgraph CORE["core/ — Engine only"]
        CTX["context.py"]
        STEP["step.py"]
        PLAN["planner.py"]
        RUN["runner.py"]
        PIPE["pipeline.py"]
        RES["resolver.py"]
        EVENTS["events.py"]
        PROD["products.py"]
    end

    subgraph LIB["lib/ — Shared plumbing"]
        ENV["env.py"]
        UTILS["utils.py"]
        PATHS["paths.py"]
        NOTIFY["notify.py"]
        SPARKLE["sparkle.py"]
        VERS["versions.py"]
        R2["r2.py"]
        TEST["testing.py"]
    end

    subgraph PRODUCTS["products/ — Product data"]
        SB["server_binaries.py"]
        DOCTOR["doctor.py"]
        BOS["browseros/product.py"]
        BCLAW["browserclaw/product.py"]
    end

    subgraph BUILD["steps/ — BUILD toolset"]
        SETUP["setup/"]
        COMPILE["compile/"]
        SIGN["sign/"]
        PACKAGE["package/"]
        STORAGE["storage/"]
    end

    subgraph RELEASE["release/ — RELEASE toolset"]
        RCOMMON["common.py"]
        RPUB["publish.py"]
        RLIST["list.py"]
        subgraph OTA["ota/ — OTA subtoolset"]
            OTACOMMON["common.py"]
            OTASERVER["server.py"]
            OTASIGN["sign_binary.py"]
        end
    end

    subgraph DEV["patchkit/ — DEV toolset"]
        BATCH["batch_apply.py"]
        EXTRACT["extract/"]
    end

    CLI --> CORE
    CLI --> LIB
    CLI --> RELEASE
    CLI --> BUILD
    CORE --> LIB
    BUILD --> CORE
    BUILD --> LIB
    BUILD --> PRODUCTS
    RELEASE --> LIB
    RELEASE --> CORE
    DEV --> LIB
    DEV --> CORE
    PRODUCTS --> CORE
    OTA --> LIB
Loading

Reviews (2): Last reviewed commit: "chore(browseros): fix stale versions pat..." | Re-trigger Greptile

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