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
Conversation
… pinned Node
Nothing here had been run on Windows, and both steps died before staging a
byte:
* `npm` is `npm.cmd`, and since the 2024 argument-injection fix Node will
not execFile a batch file at all — EINVAL, on a machine with npm
installed perfectly well.
* GNU tar (what Git for Windows puts first on PATH) parses `-czf C:\...`
as the remote spec `host:path` and aborts. System32's bsdtar does not,
so which shell invoked the build decided whether recording packaged.
Paths now go relative to an explicit cwd, which both tars read alike.
And the runtime the sidecar runs ON is now pinned on Windows too, not copied
from `process.execPath`. macOS already downloads 22.17.0; Windows shipped
whatever Node ran the build — 22.14 here, 24 from release.yml, something
older on the machine whose build could not resolve a verbatim path at all.
That is how a path-resolution bug hides in the build machine instead of in
the code. win-x64 publishes a bare self-contained node.exe, so there is
nothing to unpack.
Verified on Windows 11 with GNU tar first on PATH: prepare downloads the
pinned 22.17.0 and the win32 SDK, package stages a 37.0 MiB archive with a
win32/x64 manifest, agent-windows.exe and the SDK's own node_modules inside.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ss sees it Tauri answers `resource_dir()` with `\?\C:\...` on Windows. Every Rust-side check passes on it, which is why nothing catches this: the path is valid to Rust and to Windows. It reaches Node twice — the sidecar's script argument and RECALL_SDK_PATH — and an older Node parses it as a UNC share, takes `C:` for the host, and lstats it. That is `EISDIR: lstat 'C:'`, the sidecar dying before it runs a line of its own code, and it is what "Record does nothing on Windows" was in the reference app (brains-desktop PR #12, commit 0e06e45). Honest scope: on the Node this now pins (22.17.0) I could NOT reproduce the crash — an A/B against the real binaries loads the sidecar and requires the SDK from a verbatim path fine, on both forms. Node fixed it somewhere between the version that build shipped and this one. So this is one line of insurance that removes a variable rather than the crash-fix it was upstream; the pin in the previous commit is what actually makes the behaviour deterministic. Both are cheap and neither is speculative — the same path also reaches the browser engine and the manifest loaders. Simplified once, at the seam where Tauri hands the path over, rather than at each of the places that eventually spawn a child. A genuine UNC path has no drive letter to misread and is left alone (dunce's rule, not ours). dunce was already in the lockfile via the Tauri tree; this only makes it a direct dep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file's header calls "the group, not the process" load-bearing: Recall launches a native helper beneath Node, and an orphan of it deadlocks the next app launch. Both halves of that were `#[cfg(unix)]` — process_group(0) at spawn and kill(-pid) in Drop — so on Windows nothing in our code could reach the helper at all. Windows has no process group to signal; the primitive is a Job Object, where membership is inherited by everything a member starts and KILL_ON_JOB_CLOSE has the OS terminate all of them when the last handle closes. WHAT I MEASURED, because the header's warning invites a stronger claim than the evidence supports: against the real SDK on Windows 11, `prepare` does launch a real agent-windows.exe (0 → 1 helpers), and dropping the sidecar does clear it. But with the job object disabled the helper ALSO disappeared, ~195ms later, indistinguishable from with it — Node is killed abruptly (TerminateProcess, no exit handling), so the helper is noticing its parent died on its own. The leak this fix is named for did not reproduce. Kept anyway, and the honest justification is narrower than "fixes a leak": the reason a caller discards a sidecar is a WEDGED native SDK, and a wedged helper is precisely the one that may not notice anything. Self-exit is the SDK's courtesy; the job is the OS's guarantee. It costs one handle. The handle is a FIELD rather than a local so it closes after Drop's explicit kill, and so a force-killed app still takes the tree with it (the OS closes handles for a dead process). Assignment happens immediately after spawn, before the child is asked to do anything: Node starts the helper while handling `prepare`, which cannot happen until spawn returns and a caller writes, so nothing it spawns is born outside the job. A job the OS refuses is logged and the sidecar still runs — no recording is worse than a leaked helper. Two unit tests, both non-vacuous on Windows: the child really is a member (IsProcessInJob — a silent assignment failure would make every claim above false), and closing the handle KILLS a child nobody killed, which is the only way to know the limit flag took effect rather than merely being set. Also fixes a Layout assertion that could not pass on Windows: the sidecar runtime is `recall-node.exe` there, and `ends_with` on a Path is by component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Recording had never been executed on Windows (the engine README said so), and nothing in the suite could have caught what actually broke it in the reference app. `fake_sidecar.rs` is `#![cfg(unix)]`, so on Windows it does not run at all; `test-sidecar.mjs` drives the sidecar against a fake SDK. A path the app resolves correctly and Node cannot, and a helper that outlives its parent, are invisible to both — only real bytes show them. So: the post-install smoke check the reference app's review argued for, in the cheapest form that still runs real code. It needs no API key (the key buys uploads and transcription; extraction, the sidecar and `prepare` are entirely local) and no microphone (`prepare` warms a capture, it does not start one). Ignored by default because it needs a built bundle's assets, which it takes from two env vars and names in the failure when they are unset. What it proves, on this machine today: the 38 MiB archive verifies and unpacks, the real recall-node starts the real sidecar, `init` and `prepare` reach Recall's native Windows recorder (which reports microphone and system-audio granted), a real agent-windows.exe appears, and none survives the teardown. It also refuses to prove things vacuously: it counts helpers BEFORE and DURING, and if `prepare` never launched one it says the orphan check established nothing rather than reporting a pass. That distinction is the whole reason the measurement in the previous commit could be made at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three of the four tests failed there, and none for a Windows reason. Two fixtures fired their events from inside requestPermission(), which the sidecar only calls on darwin — so a test about platform-independent detection logic (trust an ID-only meeting event over whole-desktop capture) was silently macOS-only, and the stderr-not-stdout rule went unchecked. The events now come from prepare, which runs everywhere. The permissions assertion was the one real platform difference: requestPermissions() returns early off darwin, so nothing is requested and nothing is reported. Asserted as the Windows contract now instead of failing — worth knowing that Windows recording consults no permission of its own, though the live smoke test shows the SDK reporting microphone and system-audio granted without being asked. Not cosmetic: the failures left children alive, so the suite ran 120s and produced no output at all before the runner gave up. It is 4.6s green now — which matters, because a gate nobody can run on the platform being changed is how this class of bug got in. Audit ledger updated with what was measured rather than what was expected: both Windows rows move out of N/A, and both record that the failure they name did NOT reproduce here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The engine README said "the code paths are there; none of it has been run." That is no longer true: on Windows 11 with a keyed build, record → WAV → upload → Recall → transcript works, rendered with speaker labels. What the run found is worth more than the fact that it passed. The first two captures were 21 seconds of nothing, and the app could not tell: default input, disconnected, +30 dB boost peak -34.9 dB spread 1.4 dB the same device idle, nobody speaking peak -35.9 dB after switching the default input peak -11.5 dB spread 29.5 dB Twelve seconds of loud speech was indistinguishable from that device's idle hiss — measured against ffmpeg on the same machine — because nothing was plugged into it. Recall bound the system default correctly; the default was deaf. Both takes cleared `hasAudio`, uploaded, and billed a transcript job. So the Windows gap is not capture, it is that nothing probes the DEVICE: no default-mic watcher, no probe feeding `micDeviceMismatch`, and a silence gate that a boosted dead input walks straight through. Level cannot catch that. Variance can — 1.4 dB against 29.5 dB — and `window_peak` is already tracked, so it needs no new platform code. Written down where the next person looks rather than left as tribal knowledge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g reads it `AudioStatus` was defined in `call_mic.rs` — a platform module — without `#[serde(rename_all = "camelCase")]`, while every sibling wire type in `status.rs` carries it. So the backend answered `window_peak` / `default_device` / `call_app_device` and the client read `windowPeak` / `defaultDevice` / `callAppDevice`: three `undefined`s, on every platform, since the feature was written. `micDeviceMismatch` could never return true, and `micMismatch` was therefore set nowhere and rendered nowhere. This is not a Windows bug; macOS has been carrying it just as long. No test constructed `AudioStatus`, and `recording_audio_status` was missing from the op-table smoke list, which is why nothing anywhere noticed. The type MOVES rather than just gaining the attribute. Leaving it in a platform module fixes today's instance and leaves the trap armed for the next type defined next to a platform query; `status.rs` is where `rename_all` is the default a reader can rely on. `call_mic::audio_status(peak, window_peak)` stays behind as the platform-aware constructor, so the device lookups do not move anywhere. The refresh stays UNARMED, deliberately. Fixing the casing arms `recording_refresh_audio` for the first time in the field, and that call pauses and resumes a live capture — on macOS, which nobody here has hardware to verify a live pause/resume on. The 2-observation detection and the latch stay; the invoke is replaced by a pill title that NAMES both disagreeing devices, which is the part a user could act on anyway. Arm it after one verified macOS run. Tests: a literal specimen asserting the three keys the client actually reads, plus a sweep over one specimen of every type that crosses the IPC boundary asserting no key survives in snake_case. The sweep is the structural guard — a new wire type that forgets `rename_all` now fails at the seam instead of silently disabling whichever client branch reads it.
… sees zero The window peak is CONSUMED by reading it: the sidecar returns `windowPeak` and zeroes it in the same breath (`sidecar/index.cjs`), which is what makes it the only value that can tell a dead microphone from a loud moment that already happened. Two callers were reading it. `recording_status` and `recording_audio_status` both fire on the same one-second tick, and `audio_status()` issued a second consuming `peak` RPC of its own — so whichever landed second read ~0. A perfectly loud call reported silence on some ticks, and which reader lost was a race. `audio_status()` becomes the echo: it reports the values `peaks()` last stored and issues nothing. `peaks()` is now the single reader, which is also what makes `last_window_peak` a live field — it was written and never read. On the client, `#checkAudioStatus()` moves into the `.then()` chain after `refreshStatus()`. With the echo in place, firing them concurrently would hand a tick the PREVIOUS tick's window; ordering makes it guaranteed rather than merely likely. The fake sidecar now models read-and-reset — its `peak` arm returns the window once and 0 afterwards — and the existing capture-lifecycle test asserts both halves: the second reader on a tick still sees 7, and no extra `peak` RPC was issued. No test is added, so the suite's pinned "4 passed" holds. `record-pill.test.ts` gains the `recording_audio_status` stub it never had. Unstubbed, it threw on every tick and the store's empty catch swallowed it — so this path was untested from both ends at once: the backend shape was wrong (previous commit) and the mounted harness never exercised the reader. Flagged: `fake_sidecar.rs` is `#![cfg(unix)]`, so this fix has no Rust coverage on the platform the bug was found on. The mounted test covers the wiring everywhere.
`SILENCE_PEAK` catches an input feeding zeros. It cannot catch a disconnected
one with gain on it, and that is what cost a call. A disconnected USB line-in
at +30 dB boost, recorded over twelve seconds of loud speech:
default input, disconnected, +30 dB peak -34.9 dB RMS -48.1 dB spread 1.4 dB
same device idle, nobody speaking peak -35.9 dB RMS -48.3 dB —
after switching the default input peak -11.5 dB RMS -29.2 dB spread 29.5 dB
The first two rows agree to within 1 dB: speech was indistinguishable from
that device's own hiss BY LEVEL. The app recorded 21 seconds across two takes,
marked both as having audio, uploaded both, billed two transcript jobs that
came back empty, and told the user nothing. Level cannot see this. Variance
can — speech moves, hiss does not.
Measured in two places, split by job, and deliberately NOT in the sidecar (no
protocol change, no policy in JS, and its accumulators die with a discarded
sidecar):
* FINALIZE, in `library.rs` — the durable verdict that gates transcription
and filing. It reads the finished file, so it depends on neither the UI's
poll nor the sidecar's liveness, and it is the same path the index
self-heal already uses, so a fresh capture and a legacy one now agree.
They did not before: stop took `has_audio` from the sidecar's CUMULATIVE
peak while self-heal probed the file.
* LIVE, in `guard.ts` + the store — the ~60 s warning, off the window peaks
the store already receives once a second. Judged on the ring, never on
`status.peak`: a cumulative max never falls, so one blip at capture start
would suppress the warning for the rest of the call — the same trap
`guard.ts` already documents for the level warning.
The verdict is conjunctive with a level band, never variance alone:
flat_input ⟺ windows ≥ 40 ∧ floor > 0
∧ SILENCE_PEAK < ceiling < FLAT_CEILING_PEAK
∧ spread_db(floor, ceiling) < 6.0
6 dB is 4.3× the measured dead spread and about a fifth of the measured speech
spread; there is nothing observed between 1.4 and 29.5 to be careful about.
The lower bound hands digital silence back to the gate that already owns it.
The upper bound is the loud-steady-tone exemption and is THE ONE CONSTANT WITH
NO MEASURED NEGATIVE CASE BEHIND IT — capture hold music at the next bring-up.
`probe()` becomes a projection of a new `assess()`; 40 × 8 KiB windows replace
5 × 64 KiB at the same 320 KiB. That also fixes a real bug: five fixed points
at 10/30/50/70/90% meant a long call whose speech fell between them read as
SILENT. The early exit goes — a spread needs every window.
`has_audio` keeps its meaning and `flat_input` is a sibling. A boosted dead
input is not silent, and a SILENT badge would send the user hunting for a
muted microphone instead of a disconnected one, so the badge is NO INPUT and
the pill's warning is "⚠ No input". `IndexEntry.flat_input` carries
`#[serde(default)]`, which is load-bearing: `read_index` swallows a parse
failure into an EMPTY MAP, so without it every existing user's library would
silently vanish rather than error.
Both billing doors close: the post-stop auto-fetch and `canFetchTranscript`.
Filing takes an explicit `flatInput` argument rather than another overload of
a peak that already means three things.
Recall binds capture to the SYSTEM DEFAULT input. When that default is the
wrong device, everything downstream still succeeds — the sidecar runs, the WAV
is written, the upload completes, the transcript job is billed — and the only
thing missing is the audio. The warning this replaces read, in full:
No audio is reaching brains — check the microphone permission or input
device.
True, and useless. On the bring-up that produced this change the answer was a
disconnected USB line-in that had quietly become the system default; naming it
would have saved the hour it cost. It now reads:
No audio is reaching brains from "Microphone (3- USB Audio Device)" —
check that device is connected, unmuted, and permitted.
New file rather than an addition to `call_mic.rs`: that file is 418 lines and
Windows COM would carry it past the 600-line gate. It also is not the same
job — `call_mic` BORROWS and restores the macOS default device; this only
asks a question.
Zero new crates. `windows` 0.61.3 is already in Cargo.lock via
tao/tauri/wry/webview2-com, so the lockfile gains exactly one line.
`windows-sys` stays for the Job Object — rewriting proven shipped code is
churn. The sequence is the one verified by hand from PowerShell on 10 Aug:
CoInitializeEx → MMDeviceEnumerator → GetDefaultAudioEndpoint(eCapture,
eConsole) → GetId → OpenPropertyStore(STGM_READ) → PKEY_Device_FriendlyName.
eConsole, not eCommunications: eConsole is what the Settings picker writes and
what the SDK binds, so eCommunications could name a device that is not the one
being recorded — worse than saying nothing.
`ComGuard` balances CoInitializeEx INCLUDING when it answered S_FALSE. S_FALSE
means COM was already initialized on this thread and it still took a
per-thread reference; skipping the uninit leaks one every capture.
DEPARTURE FROM THE PLAN, deliberate: the plan called for a `PropVariant` RAII
guard calling `PropVariantClear`, on the basis that windows-rs 0.61's
PROPVARIANT has no Drop. It does — `windows-0.61.3/src/extensions/Win32/
System/StructuredStorage.rs` implements Drop as PropVariantClear. Adding the
guard would have been a DOUBLE FREE. The type's own Drop is left to do it, and
the name is read through its `Display` (PropVariantToBSTR) rather than by
reaching into the union by hand.
Privacy, deliberate: the friendly name is shown and logged; the ENDPOINT ID
never leaves the process. It is a stable per-device identifier, v1 refused to
log it, and that judgement stands — it is fetched only as a future watcher's
comparison key.
It can never break recording: `Option<String>`, every step `.ok()?`, and no
unwrap/expect/panic in the file. A machine with no capture endpoint — every CI
runner — answers None and the UI shows nothing extra.
The deciding logic is platform-free so it is covered on every CI leg, not just
the one that does not exist yet: `display_name`, `state_word` and `is_suspect`
are ordinary functions with ordinary tests. Behind the Windows cfg only what
is assertable without knowing the machine: that the query is stable across
calls and survives a short-lived thread (which is the S_FALSE balance).
Verified on this Windows 11 machine, where it returns "Microphone Array
(Intel® Smart Sound Technology for Digital Mic…" — the ® being exactly why
truncation is on a char boundary and not a byte one.
The device also appears as a chip beside the warning in the titlebar, bounded
at 18ch and ellipsised so arbitrary vendor text cannot push the titlebar's
other columns around, and only while WARNING — it is dead weight on a healthy
call. On the plan's open question of whether that chip moves an
`eval:pixels` region: the pixel eval CANNOT answer it on this base. It is
already BLOCKED on all four surfaces (goldens self-marked UNUSABLE after the
W1 gaps refactor, plus missing fixtures for set_overlay, set_tray_status,
set_tray_recording_visible and recordings_list_paged). The chip is gated on
`pill.warn`, which requires a live capture the static fixtures do not have, so
it cannot appear in those frames regardless. Re-check when the goldens are
re-recorded.
Call recording had never been run on Windows, and the reason it could go that
long is that nothing in CI ever compiled for it — `ci.yml`'s matrix was
`os: [macos-latest]`. That is the root cause of the whole saga, not a
footnote to it.
Two gaps, both load-bearing:
* NO WINDOWS LEG AT ALL. New `windows` job: clippy with `--all-targets`
(without it the test-only code is not even compiled), the unit tests, and
a build. `defaults.run.shell: bash` because every step here is POSIX and
the runner's default is pwsh.
* CLIPPY NEVER REACHED THE ENGINES. `-D warnings` only ever covered
`brains-desktop`, so no engine crate's warnings gated anything. Added to
the macOS leg too — these gaps are why the defects survived.
That second one found SIX errors in `brains-recording`, not the two the plan
expected, and none are `#[allow]`ed:
* `call_detector.rs` — an unused `let config` in a test. Fires on macOS too;
it survived because clippy never looked here.
* `library.rs` ×3 — `io::Error::new(ErrorKind::Other, e)` → `Error::other`.
* `library.rs` ×2 — a manual `is_multiple_of` and an `i32 as i32`, both in
the new detector tests.
* `sidecar.rs` — `impl Drop for Job` sat AFTER the test module. Moved up
beside the rest of `Job`'s behaviour, which reads better anyway.
`src-tauri/src/lib.rs` also had `CallDetector`/`DetectorConfig` imported at the
top and used only inside the macOS block. They move to a local `use` INSIDE
that block: the file is at the 600-line hard ceiling with exactly one line of
headroom (`check-file-sizes.mjs`), and a top-level cfg-split costs two and
fails the Frontend job. It is now at exactly 600.
THE JOB IS DELIBERATELY NARROW, and the scope is the design. `--workspace` is
excluded because `agents/local/src/sandbox.rs` asserts `real_path("/tmp") ==
"/private/tmp"` with no platform gate; `-p brains-desktop --lib` is excluded
because it pulls in `brains-storage`, which has its own pre-existing clippy
debt. Both are worth fixing and both are their own PR. Landing this job red
would get `continue-on-error` bolted onto it within a week — which is the
exact failure it exists to prevent. PR #12 was publicly corrected for a green
Windows job hiding 14 failing tests.
`14-recording.yaml`'s pinned test counts are relaxed to count-agnostic
patterns. One of them was ALREADY BROKEN on dev: the two mounted files hold 21
tests and the spec demanded `Tests 1[0-9] passed`. A pinned count makes
adding a test a spec failure, which teaches exactly the wrong lesson; the
`output_not_matches: FAILED` beside it is what actually guards these.
Not done, and commented in ci.yml where the next person will look: widening
the fmt gate to `cargo fmt --all --check`. The base carries ~160 fmt-dirty
sites across the workspace and the existing src-tauri-only check is already
failing on dev. That reformat belongs in its own commit, before the flag.
The engine README gains two sections and loses a stale one; the fix audit's
"still N/A" paragraph is replaced by what is closed and what is not.
The README now carries the measured table the thresholds were derived from,
where the verdict is taken (finalize reads the file, live reads a ring — and
why neither is in the sidecar), the conjunctive rule in full, and the WASAPI
sequence with the two things a rewrite drops: the S_FALSE balance, and the
`PropVariantClear` guard that must NOT be ported because windows-rs 0.61's
PROPVARIANT already implements Drop.
The four limits are stated as limits rather than left implicit:
* `FLAT_CEILING_PEAK` was reasoned to, not measured — it is the one constant
with no negative case behind it.
* No default-mic watcher: a default wrong at capture start is caught, one
that changes mid-call is not. The README's own claim that "no race is
possible" holds BECAUSE there is no device thread — porting one makes it
false.
* No call-app device source on Windows, so the mismatch warning cannot fire
there at all. The DEFAULT device is named; the CALL APP's is not.
* The mid-capture refresh is unarmed on purpose, pending one verified macOS
run.
The "Windows: still not ported" list from b16ea90 said all three gaps were the
same missing piece. Two of the three are now closed, so it says so instead of
continuing to describe the app as it was three commits ago.
`transcriptBadge` gained NO INPUT, and the rail rendered it — in the neutral badge style, with an un-struck microphone glyph and a row tooltip offering to fetch the transcript that the flat-input gate had just refused to fetch. A dead input is the same CLASS of failure as silence: nothing usable was captured, no transcript will be fetched, and nothing will be filed. So it gets the same treatment SILENT gets — the struck-through glyph and the warning badge colour — and its own tooltip saying what happened and that no transcript was fetched. Only the WORD stays different, which was the point of having a second badge at all: "SILENT" sends someone hunting for a muted microphone, and this device was neither muted nor silent.
Amir-SSVLabs
changed the title
…rst sort The Windows job went red on its first run, which is exactly what it is for. `list_paged`'s newest-first sort tripped `clippy::unnecessary_sort_by` — a pre-existing line, newly gated because this PR points clippy at the engine crates for the first time. `sort_by_key(Reverse(created_ms))` says the same thing, and the newest-first test is unchanged and still passes. WHY IT DID NOT REPRODUCE LOCALLY: CI takes `dtolnay/rust-toolchain@stable`, which is floating, and stable had moved to 1.97.1 while this machine was on 1.93.0. The lint does not fire on 1.93. Verified after `rustup update stable`, so local and CI now agree — clippy 1.97.1, zero errors on `-p brains-recording --all-targets`. Worth knowing about the new job: a floating toolchain means a future stable can turn it red without anything in this repo changing. That is the cost of not pinning, and it is the right trade for a leg whose whole purpose is to notice things nobody looked at — but the first person it surprises should not have to rediscover why.
Three conflicts, all small: * `library.rs` newest-first sort — BOTH SIDES MADE THE SAME FIX. dev hit `clippy::unnecessary_sort_by` independently, which confirms it was a real pre-existing lint surfacing from the floating stable toolchain and not something this branch introduced. Took dev's spelling verbatim so the line cannot conflict again. * `lib.rs` module list — `pub mod remote;` (dev) and `pub mod resources;` (this branch, from bbc23a8) landed on the same line. Kept both. * `lib.rs` imports — kept dev's new `brains_native` import AND this branch's narrowed `brains_recording` one. The narrowing is load-bearing: 34cc669 moved `CallDetector`/`DetectorConfig` into a local `use` inside the macOS block so they are not unused on Windows, and that local `use` survived the merge intact (lib.rs:563). `src-tauri/src/lib.rs` is now size-lint EXEMPT at 636 lines, so this branch's one-line-headroom dance against the 600 ceiling is moot. Left as-is; the import still belongs where the code that uses it lives. Verified after the merge, on Windows: `cargo test -p brains-recording` 65 passed, `cargo clippy -p brains-recording --all-targets -D warnings` clean, `lint:size` clean, `svelte-check` 0 errors / 0 warnings across 725 files, the recording client suites 87 passed and the mounted suites 32 passed. NOT verifiable here, and not this branch's doing: `cargo test -p brains-desktop` no longer COMPILES on Windows. `src-tauri/src/browser_host.rs` (dev's, byte-identical to origin/dev, no cfg gates, declared unconditionally at lib.rs:27) calls `window.ns_window()`, which is macOS-only. So the brains-desktop half of this branch's verification now runs only on the macOS leg. That is the exact failure mode this PR is about — a macOS-only CI merged Windows-broken code — and the Windows job added here is scoped to `brains-recording`, so it does not catch it either.
1 task
dev reformatted the recording engine (rustfmt) and independently silenced the unused `config`. Every conflict was therefore this branch's SEMANTICS against dev's FORMATTING of the same lines, so the resolution is uniform: keep the semantics, then re-run `cargo fmt` to adopt dev's formatting. The crate is now fmt-clean for the first time (0 diffs), which it has never been on this branch. Two resolutions needed judgement rather than that rule: * `lib.rs` — took dev's reformatted `call_detector` export block, but DROPPED its `pub use call_mic::AudioStatus;`. 50e9607 moved that type into `status.rs` precisely because a platform module is where `rename_all` gets forgotten, and it is already re-exported from `status` two lines below. Keeping dev's line would have re-exported a type that no longer lives there. * `call_detector.rs` — dev renamed the unused binding to `_config`; this branch deletes it. Kept the deletion: the value is genuinely dead, and `_config` only silences the lint rather than removing what triggered it. Verified on Windows after the merge: `cargo test -p brains-recording` 65 passed, clippy `--all-targets -D warnings` clean, `cargo fmt --check` clean, `lint:size` clean, recording client suites 87 passed.
Review catch (@sebastian-ssvlabs): the two `#[cfg(windows)]` tests in `resources.rs` are the regression coverage for the verbatim-path bug in bbc23a8, and they execute NOWHERE in CI. The macOS leg compiles them to nothing; the `windows` leg is scoped to `-p brains-recording` and never builds this crate. That is the same "never ran off-macOS" class this PR is otherwise about, one crate over, and it should not be left for a reader to discover. The gate itself is correct and stays: `dunce::simplified` is the identity function off Windows, so these assertions cannot hold there, and rewriting them platform-free would assert against a reimplementation of dunce instead of the code that ships — a test that passes without proving anything. So the note says what is true, at the tests themselves: they run nowhere in CI, what would make them run (the Windows job compiling this crate, once `browser_host.rs` stops calling macOS-only `ns_window()` ungated), and that today the breakage means even a by-hand `cargo test -p brains-desktop resources` cannot run on Windows. Both were re-checked green on Windows 11 for this commit by cfg-gating that one `ns_window()` call locally and reverting it — which is also the evidence that gating it is the whole of the fix that crate needs.
Three conflicts, and two of them were dev correcting this branch rather than colliding with it. * `scripts/dev/recall/package-runtime.mjs` — dev's #55 ("never copy a signed framework") REPLACES what a9b7332 did here. This branch staged a COPY of the SDK and tarred that; copying a macOS framework rewrites the symlinks its signature depends on and Apple rejects the result. dev's in-place approach is correct and this takes it wholesale. But dev's rewrite calls the host tools the way that does not work on Windows: `execFileSync("npm", …)` (npm IS npm.cmd, and a .cmd needs a shell) and `tar -czf <absolute path>` (GNU tar, first on PATH under Git for Windows, reads the `C:` as a hostname and aborts before touching the disk). Both are exactly what a9b7332 fixed. So the resolution keeps dev's semantics and routes the two invocations back through `host.mjs`, where those two facts already live. Verified by running `npm run recall:package` on Windows 11: it takes the real path and stages 37.0 MiB, rather than dying at either step. * `scripts/eval/specs/14-recording.yaml` — dev fixed the pinned test count too, with a better pattern: `Tests\s+[1-9]\d* passed` is robust to vitest's column alignment where this branch's literal two spaces was not. Took dev's, and dropped the now-stale comment explaining that the count HAD been broken — dev fixed it, so the history no longer needs narrating. * `scripts/eval/RECORDING-FIX-AUDIT.md` — deleted by dev's #60 ("stop measuring against the app we left"). Accepted the deletion rather than resurrecting a file that was removed on purpose; everything commit 382fd82 added to it about the detector, its thresholds and its four remaining limits already lives in `src/engines/recording/README.md`, which is where the rest of that commit's documentation went. Verified after the merge on Windows: `cargo test -p brains-recording` 65 passed, clippy `--all-targets -D warnings` clean, `cargo fmt --check` clean, and `npm run recall:package` completes.
Contributor
Author
Overlap with #62 — agreed sequencing: #62 firstFlagged from #62 that this PR adds a separate
#62's matrix is the better mechanism and this job should give way to it. The separate job here exists only because extending the matrix was impossible at the time — the comment in
#62 also fixes Plan, so neither of us rebases onto a moving target:
So: no rebase needed on #62's side, and nothing here reverts it. The remaining three shared files are small and this branch is already merged up to current 🤖 Addressed by Claude Code |
…arrate Hygiene pass over this branch's own diff. DEAD CODE. `input_device::state_word` and `is_suspect` were defined, tested, and called from nowhere — proven by a repo-wide search, the only references being their own definitions and their own tests. They were written so the module would have "platform-free deciding logic" exercised on every CI leg, but two of the three deciding functions decided nothing; only `display_name` has a caller. Tests over uncalled code make coverage look better than it is, which is the opposite of what that seam was for. Removed with their tests, and the module header no longer claims all three. COMMENTS. Three that narrated provenance rather than describing the code: the ComGuard doc's aside about what a rewrite tends to drop, the store's "until this commit the backend answered in snake_case", and the Windows job's citation of PR #12. The requirement, the warning and the never-continue-on-error rule all survive; only the history goes. The measured evidence behind the detector's constants stays exactly where it is — that is the reason those numbers are trustworthy, not narration. Verified: 63 tests (was 65, minus the two that tested the removed helpers), clippy `--all-targets -D warnings` clean, `cargo fmt --check` clean, the recording client suites 87 passed, and `npm run check` / `lint:size` / `lint:imports` / `lint:css-vars` each exit 0.
Amir-SSVLabs
added a commit
that referenced
this pull request
Aug 13, 2026
#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>
This was referenced Aug 13, 2026
stefan-ssv-labs
added a commit
that referenced
this pull request
Aug 13, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure this starts from
Call recording had never been run on Windows — the engine README said so in as
many words. On 10 Aug it was run end to end on Windows 11 with a keyed build:
record → WAV → upload → Recall → transcript with speaker labels.
The run found something worse than the bugs it fixed. The system default
input was a disconnected USB line-in at +30 dB boost. The app recorded 21
seconds of it across two takes, marked both
hasAudio: true, uploaded both,billed two transcript jobs that came back empty, and told the user nothing.
The first two rows are the same to within 1 dB: twelve seconds of loud speech
was indistinguishable from that device's own idle hiss. Level cannot catch a
boosted dead input. Variance can. Recall bound the system default correctly;
the default was deaf.
Chasing it exposed three defects that are not Windows-specific at all:
AudioStatuswas serialized in snake_case while the client readwindowPeak/defaultDevice/callAppDevice. Mic-mismatch detection hasbeen dead since it was written, on macOS too. No test constructed
AudioStatus, andrecording_audio_statuswas missing from the op-tablesmoke list — which is why nothing noticed.
windowPeakonread; both
recording_statusandrecording_audio_statusfired on the same1 s tick, so whichever landed second saw ~0. A loud call intermittently
reported silence, and which reader lost was a race.
ci.ymlwasos: [macos-latest]), which isthe root cause of the whole saga.
-D warningsalso only ever coveredbrains-desktop, so no engine crate's warnings gated anything.What is in here
Six Windows commits were already on the branch; the ones after them are the
general fixes.
a9b7332bbc23a82fc1d7779cc2fa4f3e044b16ea9050e9607AudioStatuscrosses the wire in snake_case, so nothing reads ita4d033e3ad819d523ba9f34cc669382fd828e10c8cNO INPUTrow looks like the failure it isThe detector
Variance is measured in two places, split by job, and deliberately not
in the sidecar (no protocol change, no policy in JS, and its accumulators die
with a discarded sidecar):
library.rs::assess— the durable verdict that gatestranscription and filing. It reads the finished file, so it depends on
neither the UI's poll nor the sidecar's liveness, and it is the same path the
index self-heal already takes, so a fresh capture and a legacy one now agree.
They did not before: stop took
has_audiofrom the sidecar's cumulativepeak while self-heal probed the file.
guard.ts::shouldWarnFlatInput+ the store — the ~60 s warning,off the window peaks the store already receives once a second. Judged on a
ring, never
status.peak: a cumulative max never falls, so one blip atcapture start would suppress the warning for the rest of the call.
Conjunctive with a level band, never variance alone. 6 dB is 4.3× the measured
dead spread and about a fifth of the measured speech spread; nothing has been
observed between 1.4 and 29.5 dB.
has_audiokeeps its meaning andflat_inputis a sibling — a boosted dead input is not silent, and aSILENTbadge would send someone hunting for a muted microphone. Both billingdoors now close: the post-stop auto-fetch and
canFetchTranscript.probe()becomes a projection ofassess(); 40 × 8 KiB windows replace5 × 64 KiB at the same 320 KiB, which also fixes a real bug — five fixed points
at 10/30/50/70/90% meant a long call whose speech fell between them read
SILENT.
IndexEntry.flat_inputcarries#[serde(default)], which isload-bearing:
read_indexswallows a parse failure into an empty map, sowithout it every existing user's library would silently vanish.
Naming the device
The highest-value line in the change. The warning read "No audio is reaching
brains — check the microphone permission or input device" — true and useless.
It now names the device. Zero new crates:
windows0.61.3 is already inCargo.lockvia tao/tauri/wry/webview2-com, so the lockfile gains one line.Things a reviewer should push on
PROPVARIANT. It called fora RAII guard calling
PropVariantClear, on the basis that windows-rs 0.61'sPROPVARIANThas noDrop. It does —windows-0.61.3/src/extensions/Win32/System/StructuredStorage.rs:32implements
DropasPropVariantClear. The guard would have been a doublefree, so the type's own
Dropis left to do it. Worth a second pair of eyes.FLAT_CEILING_PEAKis the one constant with no measured negative casebehind it. It is the loud-steady-tone exemption (hold music, a test tone)
and it was reasoned to, not measured. Flagged in code and in the README;
capture a hold-music sample at the next bring-up.
recording_refresh_audiofor the first time in the field, and that callpauses and resumes a live capture — on macOS, which nobody here has hardware
to verify a live pause/resume on. The 2-observation detection and the latch
stay; the invoke is replaced by a pill title naming both disagreeing devices.
Arm it after one verified macOS run.
fake_sidecar.rsis#![cfg(unix)], so the commit-8 test onlytypechecks on Windows. Since resolved by CI: the Eval job ran it on
macOS and it passed (
14-recording · sidecar spawn/handshake/kill-group against a fake binary, 28.9s, exit 0), so the stateful-peak assertion hasnow genuinely executed.
bodies say so. The verbatim path is fine on Node 22.17.0, and the orphaned
helper self-exits in ~195 ms with or without the Job Object. They are
insurance, not fixes for observed crashes — please don't read them as more
than that.
Merged with dev, and what that turned up
devmoved (chromium-engine #28, workstation #44) and this branch merged it.Three small conflicts, resolved in
c768369— notablydevhit the SAMEclippy::unnecessary_sort_byindependently, which confirms that lint was areal pre-existing one surfacing from the floating stable toolchain rather than
anything this branch introduced.
The merge also surfaced a live bug on dev:
brains-desktopdoes not compileon Windows.
src-tauri/src/browser_host.rs:80callswindow.ns_window(),a macOS-only Tauri API, with no
cfggate anywhere in the file, and the moduleis declared unconditionally at
lib.rs:27:It is not this branch's code (the file is byte-identical to
origin/dev) andnot this branch's to fix — what Windows should do instead is a design call for
whoever owns the CEF engine. But it is worth stating plainly, because it is
this PR's thesis happening again in real time: a macOS-only CI merged
Windows-broken code and nobody saw it. Note the
Windows (recording)job addedhere is scoped to
-p brains-recording, so it does NOT catch this either;widening it to
-p brains-desktopwould, once that compiles.Which of this PR's tests never run in CI
Because
brains-desktopcannot be compiled for Windows, thewindowsjob isscoped to
-p brains-recording. That has a consequence worth stating plainlyrather than leaving for a reviewer to discover (thanks @sebastian-ssvlabs for
pushing on it):
resources.rs::a_verbatim_resource_dir_loses_its_prefix_before_reaching_noderesources.rs::a_genuine_unc_path_is_left_aloneops.rsop-table smoke,StopResultcamelCasebrains-recordingThe first two are
#[cfg(windows)]and are the regression coverage for theverbatim-path bug in
bbc23a8— the macOS leg compiles them to nothing, andthe Windows leg does not compile this crate. The gate is correct and must
stay:
dunce::simplifiedis the identity function off Windows, so thoseassertions cannot hold there, and rewriting them platform-free would assert
against a reimplementation of dunce rather than the shipping code. They are
verified by hand on Windows and by nothing else.
resources.rsnow says so atthe tests themselves.
The fix is to widen the
windowsjob to-p brains-desktopthe momentbrowser_host.rscompiles for Windows — noted in both places.Deviations from the plan, all deliberate
Clippy found six errors in
brains-recording, not the two expected. Allfixed properly, none
#[allow]ed.The Windows job excludes
-p brains-desktop --lib, because it pulls inbrains-storage's pre-existing clippy debt. Same reasoning the plan alreadyused to exclude
--workspace(sandbox.rsasserts/private/tmpunguarded). Landing the job red is how
continue-on-errorgets bolted on —and PR fix(windows): recording is dead — verbatim-path sidecar crash + stuck-runtime upgrades #12 was publicly corrected for exactly that.
The fmt gate is NOT widened to
--all. The base carries ~160 dirtysites and the existing
src-tauri-only check is already failing ondev.That reformat belongs in its own commit; commented in
ci.yml.The pixel-eval open question is now ANSWERED — the chip moves nothing.
The plan flagged the titlebar device chip as possibly shifting a snapshot
region. It does not. Once
devrevived the goldens and pinned Chrome, thepixel job captured all seven surfaces and every one is byte-identical:
As expected: the chip is gated on
pill.warn, which needs a live capturethe static fixtures never have. No baseline was updated and none needed to
be.
Inherited CI failures — now mostly fixed by dev
This section described five failures that reproduced at the original merge
base. After merging
dev(which landedci: make the pipeline green), mostare gone: Frontend and Lint resources now PASS, and
svelte-checkreports0 errors / 0 warnings across 725 files.
What remains, and is still not this PR's:
! judge evidence invalid — vision: claude failed (null): spawn claude ENOENT. The pixel job does checkout → setup-node →npm ci → install Chrome →
eval:pixels, and never installs theclaudeCLIthe required vision judge shells out to, so it cannot pass on any branch.
The pixel comparison itself is clean (see deviation 4 above).
brains-desktopdoes not compile on Windows —browser_host.rs, dev's,see the merge section above.
ledger-net.test.tsfails on Windows only (path separators). Unrelatedpane; passes on CI's Linux/macOS runners.
Fixed by this PR:
14-recording.yamldemandedTests 1[0-9] passedwhile thetwo mounted files hold 21, so that spec was already broken. Now
count-agnostic.
Verification
On Windows 11:
cargo test -p brains-recording -p brains-desktop— 83 + 65, 0 failedcargo clippy -p brains-recording --all-targets -- -D warnings— cleannpx vitest run(recording + mounted) — 119 passednpm run test:recall— 4 passednpm run lint:size— clean (src-tauri/src/lib.rssits at exactly 600, thelast line of headroom, spent on the macOS-only import move)
npx svelte-check— 0 errorsinput_device.rswas verified against real hardware on this machine: itreturns
"Microphone Array (Intel® Smart Sound Technology for Digital Mic…"—the
®being exactly why truncation is on a char boundary and not a byte one.On CI:
Windows (recording)passes — clippy--all-targets, 65 tests,build. It is the first job in this repo ever to compile the recording engine
for Windows. It also went red on its first run over a
clippy::unnecessary_sort_bythat clippy 1.93 does not report and CI's floating stable (1.97.1) does; local
stable was updated to match before the fix was pushed.
Still to do, on hardware nobody here has: the end-to-end Windows bring-up
with the dead device as default input — confirm the pill flips to "⚠ No input"
and names it, no transcript job is created, and the row badges
NO INPUT.Carved out into their own PRs
fix(storage): a v1 install upgrading to v2 crashes before any window.read_markerparses.installed-atas JSON and hard-errors otherwise, whichpropagates into the Tauri setup hook. v1 writes a bare RFC3339 string there
and never parses it. Same identity, same
~/.brains→ every existing userwho upgrades crashes on launch, on every OS. 2 files; should go first, and
be reviewed by whoever owns the data guard.
fix(recording): "no speech found" is a state, not a loop.render()returns
"", it is cached,cached()then rejects the blank file, the stateregresses to ABSENT and re-downloads the same empty document every 15 s
forever.
🤖 Generated with Claude Code