Windows: make dev build, compile and test on Windows by Amir-SSVLabs · Pull Request #62 · ssvlabs/brains-desktop

Amir-SSVLabs · 2026-08-12T13:13:14Z

Picks up the Discord flag that dev does not build on Windows, and the failures hiding behind it. Base is dev.

What was reported

1. Won't compile on Windows. browser_host.rs called window.ns_window(), which Tauri only exposes on macOS, from a module lib.rs declares unconditionally — so the default Windows build failed regardless of whether CEF was enabled.

Fixed, but not with the suggested HWND branch. The engine behind this host is AppKit the whole way down (app_mac, clip, the objc2-app-kit views in engine.rs), so wrapping an HWND into WindowPtr would buy a compile and not a working view. The Host trait already documents "no window to parent into" as a normal answer, so None off macOS is the honest one. Gmail keeps working on Windows through the OS webview, as reported.

2. Two unused imports failing clippy -D warnings — gated under the same target_os as the CallDetector::start call they serve.

3. ledger-net.test.ts built paths with join() and compared them against a hardcoded forward-slash list. Normalized.

The structural fix

CI's Rust job was os: [macos-latest]. Nothing in the pipeline compiled for Windows, which is why all three passed review and merge. The matrix is now [macos-latest, windows-latest], scripted steps carry shell: bash (the Windows runner defaults to PowerShell, where if ! cmd; then, mkdir -p and : > are syntax errors), and the externalBin stub gets its .exe name. The bundled-Chromium type-check stays macOS-only — that backend is AppKit, so asking Windows to build it would fail on an honest absence rather than on anything wrong.

What was hiding behind them

tests/headless_sandbox.rs also failed to compile on Windows (probing_cli chmods through std::os::unix, ungated while its tests were gated), which took cargo test --workspace down entirely — so nothing after it ran either. Once it built, nine more failures. Two were real bugs, not test artifacts:

PATH composition hardcoded the Unix separator

compose_path split and joined on :. On Windows that does not merely fail to split, it splits in the wrong places, because every entry carries a drive letter: C:\Windows\system32 came apart into C and \Windows\system32, neither of them a directory. Every existence-checked entry was dropped and the composed PATH came back empty, so resolve_claude_path searched it for a CLI it could never find — and the app reported the claude CLI as missing on a machine where it was installed and on PATH. Fixed for both claude and codex, which shared the same split.

The regression test asserts through a resolution rather than a round trip, deliberately: splitting on : and re-joining on : reassembles the same string, so only a check that resolves the entries can tell the difference. It fails against the old separator.

The wipe guard looked for transcripts in the wrong place

encoded_cwds flattened / and \ and left : alone, so a Windows cwd encoded to C:-Users-me-ws — still drive-qualified. Windows Path::join replaces its base when the argument is drive-qualified, so projects.join(encoded) resolved against the current directory on C: instead of ~/.claude/projects. The guard counted zero transcripts and reported no prior use — the one answer this module exists to get right, because "no prior use" turns a wiped data dir into a fresh start instead of a recovery offer.

The suite kept its own copy of the encoder carrying the same bug, so the two agreed and the test passed while writing its fixture into whatever directory C: was pointing at — 18 of them, in the source tree, one per run. The test now calls the production encoder and asserts the fixture landed under the temp dir.

The rest were test-only

Seatbelt tests asserting macOS facts (/tmp resolving through a symlink; a root error policy() never gets far enough to return); a recall-node vs recall-node.exe assertion; a HOME lookup Windows does not set. Where a test had a portable half, only the macOS-dependent step is gated — runner_settings' live-settings claim still runs everywhere. The one check that reads sandbox::PROFILE as a string stays portable on purpose: the absence it asserts (no mach-lookup grant) is the one a careless edit reopens, and it should fail on whichever machine made the edit.

Also folded in, found on the way: a dev build on Windows never found its token (HOME-only lookup, plus a macOS-shaped keychain early-return — Windows Credential Manager has no per-binary ACL and never prompts, so that return bought nothing and cost the only source that works); tar given absolute paths, which GNU tar on Windows reads as a remote host:path spec; and src-tauri/recall-runtime/ was untracked but not ignored, ~37MB one git add -A from being committed.

Verification

Run on Windows 11:

cargo test --workspace — exit 0 (was: did not compile)
cargo clippy -- -D warnings — clean
cargo fmt --check — clean
npx vitest run — 138 files, 1659 tests, 0 failures
lint:size, lint:imports, lint:manifests, lint:css-vars, lint:resources — clean

Behaviour on macOS is unchanged: every gate added is target_os-shaped, and the PATH separator resolves to : there.

Branch is 6 commits behind dev and not rebased, so the merge check runs against current dev.

🤖 Generated with Claude Code

Windows is a shipping platform and nothing in this pipeline compiled for it. The cost was exact: `browser_host.rs` reached for `ns_window()` — macOS-only — in a module `lib.rs` declares unconditionally, and the default Windows build stopped linking. Every job was green through review and merge. `shell: bash` on the scripted steps, because the Windows runner defaults to PowerShell where `if ! cmd; then`, `mkdir -p` and `: >` are syntax errors, and that image ships Git Bash. The externalBin stub gets its `.exe` name there. The bundled-Chromium type-check stays macOS-only: that engine's backend IS AppKit, so asking Windows to build it would fail on an honest absence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…es anyway `ns_window()` exists only on macOS, and this file is compiled on every platform because `lib.rs` declares the module unconditionally — so without a gate the default Windows build did not compile at all, whether or not CEF was enabled. `None` off macOS rather than an HWND: the trait already says a window that is not there is a NORMAL answer, and there is no NSWindow to parent into off macOS. The backend behind this host is AppKit throughout, so handing it an HWND would buy a compile and not a working view. Gmail keeps working on Windows through the OS webview, which is the path `cef::is_ready()` already falls back to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`CallDetector` and `DetectorConfig` are used only by the macOS-gated `CallDetector::start` below, so off macOS they were two unused imports — a warning, and a hard error under CI's `-D warnings`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…o avoid Two halves of one bug, both of which left a dev build signed out. The dev-token file was looked up under `HOME` alone. That is a Unix name, set on Windows only when something like Git Bash synthesises it — a GUI launch has just `USERPROFILE`, so the lookup resolved to nothing and skipped the dev token: the same binary worked under `npm run dev` from bash and not when double-clicked. Reading both is what makes this agree with `brains_storage::default_root()`. The early `return Ok(None)` is now macOS-only, and the gate is the point. The dialog it exists to avoid is a macOS mechanism — keychain items carry a per-binary ACL there, and every rebuild is a new binary. Windows Credential Manager has no such ACL and never prompts; a read is silent. So off macOS that return bought nothing and cost the only source that works, and a Windows dev build with a token sitting in Credential Manager reported "no brains token" on every launch — signed out of boards, mini-sites and the MCP tool list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

GNU tar — what Git for Windows puts first on PATH — reads an absolute `C:\...` argument as a `host:path` remote spec and aborts ("Cannot connect to C:"). bsdtar, which macOS ships, does not care either way, so relative paths plus an explicit `cwd` is the one form that works on both. This runs inside `beforeBuildCommand`, so getting it wrong failed the whole build rather than just the recording assets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…le without it `:` was hardcoded as the PATH list separator. That does not merely fail to split on Windows — it splits in the WRONG PLACES, because every entry carries a drive letter. `C:\Windows\system32` came apart into `C` and `\Windows\system32`, neither of them a directory, so every existence-checked entry was dropped and the composed PATH came back empty. `resolve_claude_path` then searched it for a CLI it could never find, and the app reported the CLI as missing on a machine where it was installed and on PATH. One constant, used by both providers — codex already shares `find_in_dirs` from here and split its own PATH the same wrong way. The regression test asserts through a resolution rather than a round trip, and that is deliberate: splitting on `:` and re-joining on `:` reassembles the same string, so only a check that resolves the entries can tell the difference. It fails against the old separator; the three existing compose tests spelled the separator into their fixtures and now ask the platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`encoded_cwds` flattened `/` and `\` and left `:` alone, so a Windows cwd encoded to `C:-Users-me-ws` — still drive-qualified. Windows `Path::join` REPLACES its base when the argument is drive-qualified, so `projects.join(encoded)` did not join: it resolved against the current directory on C:. The guard counted transcripts somewhere that was never the CLI's project directory, found none, and reported no prior use — the one answer this module exists to get right, because "no prior use" turns a wiped data dir into a fresh start instead of a recovery offer. `C-` is what the CLI itself writes: `C:\Repos\brains` becomes `C--Repos-brains`, the colon and the separator each contributing one dash. The suite kept its OWN copy of the encoder, which is how this hid: the copy spelled the colon straight through exactly as the code under test did, so the two agreed and the test passed while writing its fixture into whatever directory C: was pointing at — 18 of them, in the source tree, one per run. The test now calls the production encoder and asserts the fixture landed under the temp dir. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Same reason `keychain.rs` reads both: `HOME` is a Unix name Windows sets only under Git Bash, so on a GUI launch the guard's home lookup answered "no home" and the migration guard silently never fired. Its test asked for `HOME` too, and panicked on Windows before it could assert anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…lsewhere `tests/headless_sandbox.rs` gated its TESTS on macOS but not the fixtures they lean on, and `probing_cli` writes a `#!/bin/sh` and chmods it through `std::os::unix` — which does not exist off Unix. The binary failed to build and took `cargo test --workspace` down with it, so nothing behind it ran either. Behind it were four more. All of them reach `sandbox::policy()`, which off macOS is `Err(Unsupported)` by design — there is no seatbelt to ask, and a scheduled run fails closed rather than running unconfined. So they were not finding a broken contract, they were staging one the platform never reaches: * the two `sandbox::tests` are built on macOS facts — `/tmp` resolving through a symlink, and a root error that `policy()` never gets far enough to return; * `actor_runner`'s stages a SPAWN failure, which means getting past the sandbox first; the earlier refusal has its own cover in `engine::tests`; * `runner_settings`' is gated at the LAST STEP ONLY. The live-settings claim above it holds everywhere and now keeps running everywhere — gating the whole test would have thrown away the portable half to buy nothing. What stays portable is the one check that reads `sandbox::PROFILE` as a string. That is deliberate: the absence it asserts — no `mach-lookup` grant — is the one a careless edit reopens, and it should fail on whichever machine made the edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`node_path()` already appends `.exe` there; only the test asked for the Unix name. `Path::ends_with` matches whole components, and `recall-node` is not `recall-node.exe`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The walk built paths with `join`, so the seven builders came back `src\layout\…` on Windows and failed against a list written the way every other path in this tree is written. Same files, same content, one character. A phantom failure only a Windows dev sees is worse than no net at all, because it teaches them to distrust the one that works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ignored ~37MB of generated SDK archive, and the one staging directory the list above it missed. Untracked is not ignored: it sat in every `git status` waiting for one `git add -A` to commit the vendored SDK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

dev's full-repo hygiene sweep (#49) and the per-app workspace work (#48) touched five of the files this branch changes. Conflicts resolved by keeping dev's shape and re-applying only the Windows fix on top: * claude/path.rs — dev's rustfmt shape, `PATH_SEP` instead of `:` * settings.rs — dev's `is_legacy_relic` naming and shortened comment, plus the `USERPROFILE` fallback * keychain.rs — the `dev_root()` helper this branch adds, which is what the other two call sites already use * actor_runner.rs — dev dropped that test's doc comment; kept the drop, kept the macOS gate * runner_settings.rs — dev's `runner_with_dynamic_workspace` rewrite taken whole, with only the `spawn_headless` tail gated Verified on Windows after the merge: `cargo test --workspace` exit 0, clippy `-D warnings` clean, `cargo fmt --check` clean, and the size/imports/manifests lints pass. One frontend test fails, and it is dev's, not this branch's: `frame/__tests__/shortcuts.test.ts > swallows nothing while the setup gate is up` times out at 5s on a clean `origin/dev` checkout too, verified in a separate worktree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two conflicts, both where #55 rewrote what this branch had touched: * `package-runtime.mjs` — #55 dropped the staged COPY entirely (copying a macOS framework rewrites the symlinks its signature depends on, and Apple rejects the result) and now tars the SDK in place. Took that whole, and re-applied only the Windows half on top: the paths stay RELATIVE with an explicit `cwd`, because GNU tar — what Git for Windows puts first on PATH — reads an absolute `C:\...` as a `host:path` remote spec. Re-checked both forms against the local GNU tar 1.35 on a synthetic tree: the absolute form still aborts with "Cannot connect to C: resolve failed", the relative one produces the same `desktop-sdk/` layout. Tarring in place is unaffected — `-C` still lands on the SDK's own parent, just spelled from the repo root. * `ci.yml` — #55 replaced the inline externalBin stubbing with `stub-bundle-artifacts.mjs`, which reads the paths out of tauri.conf.json instead of hardcoding them AND already appends `.exe` on win32. Strictly better than the inline stub this branch had, so the script wins and the Windows matrix entry stays. Also gated `actor_runner::tests::runner` and its `Path` import: #55's sibling work left `runner_settings` building its own runner, so the shared helper's only caller is now the macOS-gated test, and it was a `dead_code` warning off macOS. Re-verified on Windows: `cargo test --workspace` exit 0 and warning-free, clippy `-D warnings` clean, `cargo fmt --check` clean, size lint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`a_sidecar_that_exits_fails_the_call_instead_of_blocking` went red on this branch's macOS leg, and the branch changes nothing in this path — on macOS `PATH_SEP` is `:`, every gate added is `target_os`-shaped, and the recording change was a test assertion about `.exe`. It is a race, and it will bite the next branch just as arbitrarily. `Sidecar::call` reports a dead child at three ordered points: `try_wait` before it writes, the write itself, then the read. The fake sidecar here is `echo not-protocol; exit 0`, so it is already exiting while `call` runs, and WHICH point catches it is timing: 1. "the sidecar exited before prepare" — try_wait won 2. "send the prepare request: Broken pipe" — the write hit a closed pipe 3. "the sidecar closed while handling prepare" — the read hit EOF The assertion accepted 1 and 3. CI got 2. All three are the same claim, which is the one in the test's own name: the call FAILED rather than hung. Nothing about the recorder was wrong — only the list of accepted wordings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

stefan-ssv-labs

✅ review-pr: clean — no blockers or criticals found.

#45 merged first, which inverts the sequencing agreed in its thread (that PR was to drop its job once this matrix existed). Same outcome, done from this side: the separate `Windows (recording)` job is gone and the `rust` matrix is the one Windows mechanism. That is a real removal, not a formality, so it was checked rather than assumed. The job ran three commands and the matrix leg covers all three: cargo clippy -p brains-recording --all-targets ← already a step on dev, now runs on both legs cargo test -p brains-recording ← subsumed by --workspace cargo build -p brains-recording ← built by both of the above It only becomes redundant because of this PR: the job's own comment named `--workspace` (red on Windows via sandbox.rs) and `brains-desktop` (did not link at all) as the reasons it had to stay narrow, and both are fixed here. Its load-bearing note — never `continue-on-error`, because a job that cannot go red reads as coverage — moved onto the matrix, where the next person will look. The other three conflicts are the same story twice over: both branches fixed the same Windows bug independently. * package-runtime.mjs — both made tar's paths relative. Took dev's, which routes through host.mjs, because that helper also carries the npm.cmd fix (Node refuses to execFile a .cmd at all since the 2024 argument-injection change). Kept this branch's note that it runs inside beforeBuildCommand, so getting it wrong fails the whole build, not just the recording assets. * src-tauri/src/lib.rs — a TRAP worth naming. This branch imported CallDetector/DetectorConfig at the top under a macOS gate; dev moved them inside the macOS block instead (that file is at its size ceiling). Keeping both would leave the outer import UNUSED on every non-macOS build — a hard error under `-D warnings`, which is the exact failure this branch's comment existed to prevent. Took dev's arrangement verbatim; it is green on dev's macOS leg, which is the only leg that compiles that path. * engines/recording/src/lib.rs — both fixed the same node_path assertion. Took the union: dev's extra check that the binary resolves next to the executable, with this branch's clearer failure message. Verified on Windows 11: `cargo test -p brains-recording` 63 passed, `cargo clippy -p brains-recording --all-targets -- -D warnings` clean, and `cargo check -p brains-desktop` finishes — the claim this PR is about. `npm run lint:size` green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Amir-SSVLabs · 2026-08-13T07:39:26Z

#45 landed first — the reconciliation happened here instead

The sequencing agreed in #45's thread was: this PR's matrix lands, then #45 drops its separate Windows (recording) job. #45 merged first (b0de1d2), so the same outcome is done from this side in 27b38e0. The rust matrix is now the one Windows mechanism.

The removal was checked, not assumed. That job ran three commands, and the matrix leg covers all three:

the job's step	covered by
`cargo clippy -p brains-recording --all-targets -- -D warnings`	already a step on dev — now runs on both legs
`cargo test -p brains-recording`	subsumed by `cargo test --workspace`
`cargo build -p brains-recording`	built by both of the above

And it only becomes redundant because of this PR: the job's own comment named --workspace (red on Windows via sandbox.rs) and brains-desktop (did not link at all) as the reasons it had to stay narrow. Both are fixed here. Its load-bearing note — never continue-on-error, because a job that cannot go red reads as coverage — moved onto the matrix.

The other three conflicts were both branches fixing the same Windows bug independently. One is worth naming because keeping both sides would have broken the build:

src-tauri/src/lib.rs — this branch imported CallDetector/DetectorConfig at the top under a macOS gate; dev moved them inside the macOS block instead. Keeping both leaves the outer import unused on every non-macOS build, a hard error under -D warnings — the exact failure this branch's comment existed to prevent. Took dev's arrangement verbatim, since it is green on the macOS leg that actually compiles that path.
package-runtime.mjs — both made tar's paths relative. Took dev's, which routes through host.mjs, because that helper also carries the npm.cmd fix (Node refuses to execFile a .cmd at all since the 2024 argument-injection change). Kept this branch's note that it runs inside beforeBuildCommand.
engines/recording/src/lib.rs — same node_path assertion, both fixed. Took the union.

Verified on Windows 11 before pushing: cargo test -p brains-recording 63 passed, cargo clippy -p brains-recording --all-targets -- -D warnings clean, cargo check -p brains-desktop finishes, npm run lint:size green.

stefan-ssv-labs

✅ review-pr: clean — no blockers or criticals found.

…dev's BRNS-DESK-034 was fixed twice, independently: here, and in Amir's Windows PR (#62, commit e87d525), which reported the bug in the first place. Both do the identical thing — import relative+sep from node:path and normalize each walked path through relative(process.cwd(), p).split(sep).join("/") in place of the process.cwd().length + 1 slice. Dev's version wins outright. It names the normalization repoPath() instead of inlining it in the .map, and it carries the explanation of why a phantom Windows-only failure is worse than no net at all. This branch's copy of that file is now byte-identical to dev, so the ledger-net half of this PR is empty and what remains is BRNS-DESK-033 alone. .github/workflows/ci.yml merged clean: #62 restructured the Rust job above the Build macOS upload, which moved but did not touch it.

…nd the cache caches what cargo writes (#71) Two CI truth problems, both in the same three workflow files. ## The Windows path fix was never proven on Windows (BRNS-DESK-034, AC4) `ledger-net.test.ts` walks the tree with `join()` and compares the result against forward-slash literals, which makes it the one spec in this repository whose result depends on the path separator. It failed on every entry at once on Windows until #62 normalized the discovered side, and nothing re-proved that afterwards: the matrix #62 added is a cargo matrix, and cargo does not run vitest. The `Frontend` job that does run vitest is ubuntu-only. So the fix for a Windows bug was a claim rather than a test, and AC4 stayed open. The cheapest true closer is a Windows-gated step inside the existing `rust` job, which already installs node 22 and runs `npm ci`. The check costs seconds there; a second Windows frontend job would cost a job. It is that job's LAST step, deliberately. A failing step aborts the leg, so a frontend spec placed ahead of the cargo steps would let one red assertion delete that leg's clippy, its `--workspace` tests and its Windows link check — exactly the coverage the matrix exists to provide, erased by an unrelated failure. `fail-fast: false` guards sibling legs, not sibling steps. Running last costs nothing, because the spec's walker skips `target` and `node_modules` by name. Both flags carry weight. `--passWithNoTests=false` pins the behavior that makes the hardcoded path load-bearing — vitest exits 1 on a filter matching nothing, so a rename cannot silently retire the step. That is vitest's default today, but it is vitest's, and one line in `vitest.config.ts` would flip it for the whole repo. `--no-install` stops `npm exec` from falling back to a registry download of an unpinned vitest if the local binary ever goes missing; a step whose value is determinism should fail loudly instead. Verified locally: the spec passes 26/26, a stale filter exits 1, and `--passWithNoTests=true` exits 0 on that same stale filter, which proves the flag is read rather than ignored. Not `npm test`: that chains `drive:test` and `eval:test` behind vitest and neither has ever run on Windows, so widening to the full frontend gate would be a different change with a real chance of landing red for unrelated reasons. ## The Rust cache pointed at a directory cargo no longer writes (BRNS-DESK-035) `Swatinem/rust-cache` caches `<workspaces>/target` and defaults `workspaces` to `.`. Six sites overrode that with `src-tauri`, so the action cached a path cargo does not write: `./Cargo.toml` is the workspace root and lists the seven engine crates and `src-tauri` as members, so every artifact lands in repo-root `target/`. Removing the input at all six sites is the whole fix. The lesson is NOT that the input was always wrong, and the comments say so plainly, because the true version is the useful one. It entered at e152464 (2026-03-01), when this tree had no root `Cargo.toml` and `src-tauri` carried its own manifest and lockfile — src-tauri genuinely was the cargo root, and `src-tauri/target/` genuinely was where cargo wrote. Commit 2e0e247 (2026-08-06) added the root workspace and deleted `src-tauri/Cargo.lock` in one move, and the input silently became stale. Nothing lints a workspace-root move against the CI inputs naming the old root. That is the failure mode worth remembering, and an earlier draft of these comments asserted the opposite. The key was also wrong, in a way that is easy to miss: the action hashes `<workspaces>/Cargo.lock` only if it exists, and `src-tauri/Cargo.lock` had been deleted, so the lockfile dropped out of the key entirely and a `cargo update` stopped invalidating the entry. That is why the first run after this change re-keys and saves rather than no-opping. `save-if` on the two ci.yml jobs that run on every PR is the other half of the fix, not a separate opinion. These entries stop being a ~741 MB registry and become a real target directory, and a PR-scoped save is readable only by re-runs of that same PR while still counting against the 10 GB repository budget that evicts by last use. Writing only on `main` keeps one warm entry every PR can restore from, instead of per-PR copies nobody reads that evict the shared one and the npm caches beside it. `build-macos` is deliberately excluded: it is label-gated on a pull request and never runs on `main`, so restricting its save would mean it never writes and never restores. `dev-release.yml` gets `key: universal-apple-darwin-dev`. Both release workflows name the job `build` and run it on `macos-15`, and the action builds its key from the job id, the runner OS and arch, and the `key` input — not from the workflow name. With the same key literal, a manual `release.yml` dispatch on `dev` lands in the same ref scope with the same key, and cache entries are immutable: the second writer is silently skipped and one channel serves the other channel's target directory. That was harmless while both held nothing but the registry; it is not harmless now that they hold real builds, since the two channels pin different Recall SDKs and carry different bundle identities. Two expectations are tempered in the comments rather than overstated. A restore returns third-party dependency artifacts only — the action prunes workspace members before saving, so our own eight crates relink every run. And `release.yml` triggers on `v*` tags, where Actions caches are ref-scoped: a tag run restores from its own new ref or from the default branch, which never runs that job id, so its entries are written and in the normal flow never read. Fixing the path there is still right, but warming the release lane needs its own change and its own ticket. `dev-release.yml` is the one lane this genuinely warms, since it runs on pushes to `dev` and restores its own previous save. Verified: `actionlint .github/workflows/*.yml` exits 0, all three files parse, no `workspaces:` input remains, and the vitest spec passes under the exact command string CI will run. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…t the shell crate (#73) * style: cargo fmt --all, so the widened gate can land green (BRNS-DESK-036) Seven hunks across six files in two crates, and every one is rustfmt's own line-breaking rather than a change in what the code does. This is the commit `ci.yml` asked for in the note above its format check — "Do the reformat as its own commit, then add --all." The widening itself is the last commit here. `src/engines/model/src/codex/models.rs:5` is an import reorder; the other six are wrapping decisions. Two of those are mirror images, which is worth a word: `claude/path.rs` collapses a broken-out `ok_or(...)` back onto the call line while `codex/path.rs` expands the same construct outward. rustfmt is not being inconsistent between them — the two sites differ by the width of the error type's name, `PathError` against `CodexPathError`, and each is being moved to the shape the width rule dictates for it. Both had been hand-formatted the other way. Those two files are also why this reformat is worth more than tidiness. #62 rewrote exactly these lines to fix the Windows PATH separator and left both unformatted, because the format gate is scoped to `src-tauri`: it went green over the two files that PR had just changed, and the PR body reports `cargo fmt --check` clean in good faith. The debt here is this small only because #45 paid the rest of it — 364 of the 369 hunks that existed when the note above the format check was written. What remains is what arrived afterwards, through a gate that could not see it. rustfmt does not change semantics, and nothing here is near a macro or a conditional-compilation boundary where that generalization gets interesting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(src-tauri): two field_reassign_with_default in test code (BRNS-DESK-036) These are the only two clippy findings in the whole workspace, and both are in the one crate CI already lints. Why they survived is worth naming before the next commit widens anything, because it is not only `--manifest-path`: the clippy step also has no `--all-targets`, so even inside the single package it selects, only the lib target is linted — and both of these live in `#[cfg(test)]` code. Widening the package selector alone would have stayed green over both. Each fix is the one clippy suggests: move the single plain field assignment into the struct literal, leave everything else alone. The `set_app_enabled` calls have to remain after the initializer in both cases — they are methods rather than fields, so `..Default::default()` cannot absorb them — and `boot_tests.rs` keeps its loop over the manifest's apps for the same reason. `Settings::default()` is untouched, so both sites construct exactly the value they constructed before. That equivalence is the lint's own point: the two-step form reads as though it might not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: fmt and clippy cover the workspace, not just the shell crate (BRNS-DESK-036) `--manifest-path src-tauri/Cargo.toml` selects that package alone, and `src-tauri` is the index — CLAUDE.md's word, and it holds no domain logic. So two of the three Rust gates were pointed at the crate with the least code in it while `cargo test` had already been widened out of that exact trap, with a comment above it explaining the mechanism. Both now match the tests: `cargo fmt --all --check` and `cargo clippy --workspace --all-targets -- -D warnings`. The hint the format step echoes on failure changes with it, from `cargo fmt --manifest-path src-tauri/Cargo.toml` to `cargo fmt --all` — a failure message naming a command that reproduces nothing is its own small defect. The two gates were not failing the same way, and the difference decides which flag matters: rustfmt really was blind to the engines. It formats the packages it is given and nothing else, so all seven went unformatted — which is how #62 fixed the Windows PATH separator in `claude/path.rs` and `codex/path.rs`, left both unformatted, and watched this step go green over the two files it had just changed. Clippy was not blind to them. Cargo applies clippy's workspace wrapper to every workspace member it builds, and `src-tauri` depends on all seven engines, so their lib targets have been linted all along through the narrow step. The flag that was missing is `--all-targets`: no crate's test, bench or example target was linted anywhere, which is why both of the workspace's only two real findings were sitting in `src-tauri`'s own test code — inside the single crate the step already selected. Widening the package selector alone would have stayed green over both. That also explains the `-p brains-recording --all-targets` step this commit removes, and corrects the note that stood on it. It read "the engine crates are NOT covered by the step above", but recording was already a `src-tauri` dependency when that step was added, so the six clippy errors nobody saw cannot have been in its lib — they were test-only, which is what "tests included" in the step's own name was saying. `--workspace --all-targets` subsumes it exactly: same target selector, and `brains-recording` is one of the eight members `--workspace` resolves to. `--workspace` itself is explicit rather than load-bearing, and the review of this branch is what established that: the root manifest is virtual and declares no `default-members`, so `cargo metadata` resolves `workspace_default_members` to all eight members and a bare `cargo clippy --all-targets` would already select the same set. The flag stays to say the scope out loud, and to hold it if anyone adds `default-members` later — not because it widens anything today. The fmt step's old note refused this widening on a hunk count, and that note is the reason to keep arithmetic out of a workflow comment. At 34cc669, where it was written, the workspace measured 369 fmt-dirty hunks, 111 of them in `src-tauri`. #45 — the same PR — then paid 364 of them and nobody came back to edit the note that had just been written. On `dev` at ee2984a what was left measured 7 hunks across 6 files in 2 crates, and that is the reformat commit before this one. Every one of those figures is basis-specific; the comment now carries the mechanism and points here for the numbers. `--all-targets` leaves one hole and cannot close it, so the comment says so rather than claiming coverage it does not have. `brains-browser` declares `[[bin]] brains_cef_helper` with `required-features = ["chromium"]`, and cargo drops a target with unmet required features out of a wildcard selection silently — it errors only when the target is named. The same feature gates the engine modules inside that crate's lib, and the one step that turns the feature on is `cargo check`, not `cargo clippy`. That gated code is linted by nothing, before this change and after it; it needs its own ticket. `CONTRIBUTING.md` moves with the gate. Its pre-commit list is the only checked-in one, it promises to be "the whole safety net", and it named neither `cargo fmt` nor `cargo clippy` — survivable while the gate covered one crate, misleading now that it covers the tree most Rust changes land in. Both commands are added in the form CI runs them, with a note on why the flags are not optional. Neither gate changes which legs it runs on. Neither carries an `if:` today, so both already ran on macOS and Windows; this changes scope, not platform reach. Clippy on both is load-bearing — #62 fixed two Windows-only unused imports that failed `-D warnings`, a class of finding the macOS leg structurally cannot see. Verified locally on macOS: `cargo fmt --all --check` and `cargo clippy --workspace --all-targets -- -D warnings` both exit 0, and `actionlint .github/workflows/*.yml` is clean. Both halves of the gap were measured rather than assumed, by planting a deliberate lint and re-running both command forms: in an engine's lib the old clippy step already caught it, in an engine's test target the old step exits 0 while the new one exits 101, and for rustfmt the old step exits 0 on an engine violation the new one flags. The Windows leg's clippy cannot be measured from here, so this PR's own run is that measurement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Amir-SSVLabs and others added 14 commits August 12, 2026 15:51

Amir-SSVLabs mentioned this pull request Aug 12, 2026

audio: a dead input is one whose level never moves — the Windows run, three cross-platform defects, and a CI leg that can go red #45

Merged

stefan-ssv-labs approved these changes Aug 13, 2026

View reviewed changes

Amir-SSVLabs merged commit 17a3b21 into dev Aug 13, 2026
7 checks passed

Amir-SSVLabs deleted the fix/windows-build branch August 13, 2026 08:10

stefan-ssv-labs mentioned this pull request Aug 13, 2026

fix(ci) [BRNS-DESK-033]: the artifact paths point where cargo actually writes #65

Merged

This was referenced Aug 13, 2026

docs: the Windows tests that ran nowhere now gate, and one still does not #66

Merged

test(recording): the sidecar contract suite runs on Windows #69

Merged

This was referenced Aug 13, 2026

fix(ci) [BRNS-DESK-034, BRNS-DESK-035]: Windows runs the path spec, and the cache caches what cargo writes #71

Merged

ci(rust) [BRNS-DESK-036]: fmt and clippy cover the workspace, not just the shell crate #73

Merged

Navigation Menu

Uh oh!

Windows: make dev build, compile and test on Windows - #62

Windows: make dev build, compile and test on Windows#62
Amir-SSVLabs merged 16 commits into
devfrom
fix/windows-build

Amir-SSVLabs commented Aug 12, 2026 •

edited

Loading

Uh oh!

stefan-ssv-labs left a comment

Uh oh!

Amir-SSVLabs commented Aug 13, 2026

Uh oh!

stefan-ssv-labs left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Amir-SSVLabs commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was reported

The structural fix

What was hiding behind them

PATH composition hardcoded the Unix separator

The wipe guard looked for transcripts in the wrong place

The rest were test-only

Verification

Uh oh!

stefan-ssv-labs left a comment

Choose a reason for hiding this comment

Uh oh!

Amir-SSVLabs commented Aug 13, 2026

#45 landed first — the reconciliation happened here instead

Uh oh!

stefan-ssv-labs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Amir-SSVLabs commented Aug 12, 2026 •

edited

Loading