Skip to content

fix(windowing): use native platform chrome (BRNS-DESK-046) - #95

Merged
stefan-ssv-labs merged 5 commits into
devfrom
fix/desk-046-window-chrome
Aug 14, 2026
Merged

fix(windowing): use native platform chrome (BRNS-DESK-046)#95
stefan-ssv-labs merged 5 commits into
devfrom
fix/desk-046-window-chrome

Conversation

@stefan-ssv-labs

@stefan-ssv-labs stefan-ssv-labs commented Aug 14, 2026

Copy link
Copy Markdown

Ticket

BRNS-DESK-046

What changed

  • Removed the duplicated app.windows arrays from release and debug Tauri configs.
  • Added one Rust-owned main window specification shared by both identities.
  • Kept transparent/hidden-title chrome on macOS only; Windows and Linux retain native decorations, with the existing Windows drag/drop policy.
  • Injected a static macos | windows | linux value before frontend startup and used it to reserve traffic-light space only on macOS.
  • Added pure Rust policy/config tests and focused frontend platform parsing tests.

Requirement mapping

  • One release/debug construction path: main_window::build runs at the start of setup(); neither config declares a window.
  • Native caption policy: the builder applies transparent/hidden title only behind target_os = "macos"; decorations stay enabled everywhere and Windows disables webview drag/drop.
  • Trusted frontend signal: the initialization script injects only the compile-time platform enum; there is no IPC command, plugin, polling, user-agent check, or capability expansion.
  • Frame geometry: Titlebar.svelte keeps the existing macOS traffic-light inset and uses normal horizontal padding on Windows/Linux.
  • Drift protection: pure tests pin geometry, native-chrome branches, init-script values, config ownership, and the existing main capability target.

Validation

  • npm run check — PASS, 0 errors and 0 warnings.
  • npm run lint:size — PASS; only existing soft warnings, no new file over the limit.
  • npm run lint:imports — PASS.
  • npm run lint:manifests — PASS; both committed manifests match sources.
  • npm run lint:css-vars — PASS.
  • npm test — PASS: 143 Vitest files passed / 4 skipped, 1,728 tests passed / 17 skipped; drive 6/6; eval unit 24/24.
  • npm run build — PASS; 570 modules bundled and 6 resources staged.
  • npm run lint:resources — PASS; the six-file resource closure is exact.
  • npm exec --no-install -- vitest run --passWithNoTests=false src/layout/core/frame/__tests__/host-platform.test.ts src/layout/core/frame/__tests__/record-pill.test.ts — PASS: 2 files, 21 tests.
  • cargo fmt --all --check — PASS.
  • cargo clippy --workspace --all-targets -- -D warnings — PASS.
  • cargo clippy -p brains-desktop --lib -- -D warnings — PASS after final review wiring.
  • cargo check -p brains-browser --features chromium — PASS; only existing feature-gated warnings.
  • cargo test -p brains-desktop --lib — PASS: 127 passed / 5 ignored.
  • cargo test -p brains-desktop main_window — PASS: 3 passed.
  • cargo test --workspace with an isolated target — app/window suites PASS; the command exits 101 only for two macOS sandbox/keychain tests. The identical cargo test -p brains-local-agents --test headless_sandbox fails on origin/dev@3bca1639 with the same two SecurityServer/keychain errors (9 passed / 2 failed / 1 ignored), proving the exception is pre-existing and host-environmental.
  • npm run eval -- --no-agent — 201 PASS, 6 FAIL, 4 PENDING, 17 SKIP. The failures are pre-existing sandbox/keychain denial, no live Brains token, and the Chrome golden mismatch; no windowing assertion failed.
  • npm run eval:pixels -- --no-judge — local run BLOCKED after capturing all 7 surfaces because committed goldens use Chrome 146 while this host defaults to Chrome 152; no local pixel comparison was accepted.
  • CI Pixel eval (headless) with pinned Chrome 146 — inherited FAIL, not a branch regression. PR fix(windowing): use native platform chrome (BRNS-DESK-046) #95 reports the exact same seven-surface metrics as the unrelated BRNS-DESK-079 transport-only run (base-lineage job): start-menu/settings 0.000%, then 0.015% / 0.015% / 0.389% / 0.015% / 0.015% on the remaining surfaces with identical RMSE values. The titlebar's macOS/headless DOM and padding declaration are unchanged; only Windows/Linux receive the conditional native-caption class.
  • npm run dev:app — PASS on macOS arm64: one maximized main window painted through the debug identity with the transparent titlebar and traffic-light inset preserved.
  • npm run tauri -- build --bundles app — PASS: release binary and ad-hoc-signed brains.app bundled successfully. A launch reached native window creation, then waited on the expected real-keychain ACL prompt for the local ad-hoc identity; it was stopped without approving access.
  • cargo check -p brains-desktop --target x86_64-pc-windows-msvc — attempted, but the macOS host lacks Windows SDK headers (stdlib.h / windows.h) required by aws-lc-sys; Windows CI is the authoritative compile lane.

Platforms and gaps

  • Exercised locally: macOS arm64 debug window, frontend/browser harnesses, and packaged macOS .app build.
  • CI required: Windows Rust compile for the Windows-only builder call; the result is recorded on this PR's check suite.
  • Manual proof still required on packaged Windows 11 at 100%, 150%, and 200% scale for minimize, maximize/restore + snap, close-to-tray, native dragging, and caption/toolbar non-overlap. BRNS-DESK-047 already tracks the downstream clean-VM package/install pass.
  • Linux native decorations are represented in the pure policy test but were not compiled or launched locally.

Risks, non-goals, and shipping impact

  • Risk is limited to main-window construction and titlebar spacing. The existing label, title, geometry, maximized state, background, decorations, capability target, and close-to-tray handler remain unchanged.
  • Native Windows captions are intentional. Custom caption buttons, a general OS API, extra windows, and a frame redesign are non-goals.
  • No persisted-state migration, capability, updater, external-bin, resource-map, signing-policy, or API change. Package contents are unchanged; only window ownership moves from JSON to Rust.

Regression provenance

PR #47 documented but did not implement the remaining gap: both current configs still imposed macOS transparent-titlebar geometry on every platform, and the shared frame had no trusted platform input.

Reproduction

  • Environment: origin/dev@3bca16391fe9626767e2e6b0ac3568988fa291b0, macOS arm64 source/API inspection; packaged Windows 11 runtime proof remains above.
  • Preconditions: current release or debug Tauri configuration.
  1. Inspect app.windows in src-tauri/tauri.conf.json and src-tauri/tauri.dev.conf.json.
  2. Inspect the unconditional traffic-light inset in src/layout/core/frame/Titlebar.svelte.
  3. Launch/build on Windows.
  • Expected: Windows uses native caption controls without macOS traffic-light spacing; macOS retains its transparent frame.
  • Actual on the base: both configs request transparent hidden-title chrome and the frontend always applies the macOS inset without an OS signal.

Merge method: squash (do not merge-commit or rebase-merge).

@nir-ssvlabs nir-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving window construction out of two JSON configs into one Rust builder is the right direction, and the hazard it creates is handled: Tauri capabilities bind to a window label, so a Rust-built window under a different label would silently stop being covered by capabilities/default.json. The spec uses label: "main", the builder passes it through, and — the part I'd have asked for — a test reads the capability file and asserts windows == ["main"], so the label and the capability can't drift apart independently. The companion test asserting neither config declares a second main window closes the other direction.

The injected global is a closed loop rather than a hole: HOST_PLATFORM is a compile-time const chosen by #[cfg(target_os)], as_str() returns one of three static strings, and nothing runtime-derived reaches the format!. The frontend then allowlists the same three values instead of trusting the global, so a tampered __PLATFORM__ can't widen the type either.

  • 🟢 parseHostPlatform falls back to "macos" for anything unrecognized. That's fine for a browser dev session on a Mac, but the failure mode on the two platforms this PR exists to fix is traffic-light inset padding with no traffic lights — the exact wrong chrome, silently. "linux" (plain padding) degrades more neutrally if the injection ever doesn't land.

Checked: the label against the capability target and the tests pinning both, the init script's inputs end to end, the #[cfg(target_os = "macos")] gating on the transparent/hidden-title chrome, and that both configs really dropped app.windows. Not read: the Titlebar.svelte geometry values themselves.

Merge: ✅ into dev once the Rust jobs report — they were still pending at review time.

@nir-ssvlabs nir-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 4b58dcc3. The delta is CSS/DOM only — the label, capability coupling and init-script path I checked last time are untouched.

The rework is an improvement on the shape rather than the behaviour: instead of stamping data-platform and selecting [data-platform="macos"], the base rule now carries the traffic-light inset and native-caption removes it on Windows/Linux. That makes macOS output byte-identical to the pre-PR frame, which is the safer default when the primary platform is the one you can't afford to regress — and the comment says so at the point it matters. data-platform is fully gone from src/, so nothing is selecting on an attribute that no longer exists.

My 🟢 from the last pass still holds and is now clearly a considered trade rather than an oversight: an injection that doesn't land still yields macOS geometry on Windows/Linux. Given the base rule is deliberately the macOS one, that's the direction you've chosen, and it's defensible — worth knowing rather than changing.

Merge: ✅ into dev once the Rust jobs report.

@sebastian-ssvlabs sebastian-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-at: 4b58dcc

Comment thread src-tauri/src/main_window.rs Outdated

@stefan-ssv-labs stefan-ssv-labs left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ review-pr (self-review, cannot approve own PR): clean — no blockers or criticals found against BRNS-DESK-046.

@sebastian-ssvlabs sebastian-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-at: f64f9b7

@nir-ssvlabs nir-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at f64f9b7b. Most of the delta is a dev merge picking up #93 and #94; the PR's own commit moves dragDropEnabled from a Windows-only spec field to an unconditional .disable_drag_drop_handler().

That correction matters and I should have caught it. The deleted config had "dragDropEnabled": false for all platforms; the previous head narrowed it to Some(false) on Windows with None elsewhere, which would have re-enabled Tauri's native handler on macOS and Linux — and the native handler intercepts file drops before the webview, so the frontend's own scoped handlers would have stopped receiving them. My last two passes checked the label, the capability coupling, the chrome flags and the init script, but never enumerated the full set of keys the config lost against the builder that replaced them. That's the parity sweep this PR most needed and I ran it on the wrong axis.

I've now done it properly against the config on dev: label, title, width, height, minWidth/minHeight, resizable, fullscreen, maximized, titleBarStyle, hiddenTitle, backgroundColor and dragDropEnabled all have a counterpart in main_window.rs. Nothing else was dropped in the move.

Merge: ✅ into dev once the Rust jobs report.

@stefan-ssv-labs stefan-ssv-labs left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ review-pr (self-review, cannot approve own PR): clean — no blockers or criticals found against BRNS-DESK-046; the prior drag/drop finding is fixed in this head.

@stefan-ssv-labs
stefan-ssv-labs merged commit 29b6b62 into dev Aug 14, 2026
7 of 8 checks passed
@stefan-ssv-labs
stefan-ssv-labs deleted the fix/desk-046-window-chrome branch August 14, 2026 14:02
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.

3 participants