test(tools): pin the WSL2 PowerShell TTS fallback in voice tests - #36
Closed
spfcraze wants to merge 74 commits into
Closed
test(tools): pin the WSL2 PowerShell TTS fallback in voice tests#36spfcraze wants to merge 74 commits into
spfcraze wants to merge 74 commits into
Conversation
Whole-bug-class follow-up to the profiles/ gate: cron/, platforms/ pairing, and legacy pairing/ ran chown_hermes_tree unconditionally on every boot with the identical warm-boot cost profile. Same tree_has_non_hermes_owner gate; find evaluates the top directory first and -quits on the first mismatch, so a mis-owned tree short-circuits in O(1) while a clean tree pays one read-only walk instead of a full chown -R inode rewrite.
…iahchu-email chore: add contributor email mapping for ZachariahChu
…-sparkeros chore: contributor email mapping for sparkeros
…one-email chore: add contributor email mapping for JeffStone69
check_tool_availability runs once per managed-tool check_fn (browser, image_gen, etc.) during banner render. Each one independently triggers a ~15s blocking Nous Portal token-refresh network call when the stored token is expired. On a slow/constrained host (e.g. a small monitoring CT) that serial burst stretched startup to many minutes, appearing 'stalled'. Add a per-process memo (5s TTL) so the burst collapses into a single network round-trip. Only successful, non-forced resolutions are cached; force_fresh and insecure/ca_bundle callers bypass and don't populate the cache, so normal refresh semantics are unchanged. Verified: 3 rapid resolve_nous_access_token() calls -> 1 underlying refresh.
Follow-ups on the startup-burst memo: - Populate the memo on the valid-token fast path as well. The startup burst usually finds a VALID token, and each check_fn call still paid two cross-process file locks + state reads to reach that return; the original memo only engaged after a refresh. The token has at least refresh_skew_seconds (>=120s) of life at that return, so a 5s memo can never serve an expired token. - Clear the module-level memo in test_nous_portal_staging_allowlist's refresh-capture helper: with the fast-path populate, a token memoized by an earlier test would otherwise short-circuit the refresh these tests assert on (3 tests failed without this). - Add dedicated memo behavior tests (TTL hit, TTL expiry, insecure bypass) — the original PR shipped none. Mutation-checked: all 3 fail against main's un-memoized function, pass on this branch.
Makes interrupt-protected context compression cancellable by an explicit user or lifecycle stop, without weakening protection against ordinary incoming messages, voice interjections, or active-turn redirects. Separates explicit hard cancellation from ordinary interrupt/redirect state with a dedicated threading.Event; introduces AuxiliaryExplicitCancellation as an attempt-local frozen-cause signal; isolates the synchronous provider callback in a bounded daemon worker during protected compression; atomically linearizes Codex timeout cleanup against explicit cancellation; propagates hard cancellation through child agents and explicit stop surfaces; serializes hard-cancel admission against compression commit admission with CompressionCommitFence; aborts before session rotation or late DB commit, restores in-place transcript mutations and compressor state, and releases the heartbeat and compression lease. Based on NousResearch#74449 by @suparious. Resolved merge conflicts in agent/context_compressor.py (feasibility check + try/except) and tui_gateway/methods_session.py.
_abandon_timed_out_gateway_turn landed on main (eb4772e) after this PR's base and still used the soft interrupt(). Every other inactivity- timeout surface in this change (cron, gateway executor poll, delegate children) treats a timeout as an explicit stop that may cancel a protected compression summary — widen the same fix to this sibling.
The interactive /model picker probes the current custom endpoint live via fetch_api_models(), which defaults to a 5s timeout. A slow or flaky custom endpoint blocks the picker for up to 5s on open. The lmstudio picker probe already uses a 1.5s timeout for exactly this reason; apply the same fast-fail bound to the three custom-provider probe sites, gated on for_picker so the non-picker (5s) path is unchanged.
Three tests pin the exact kwargs of the picker probe call (test_model_switch_custom_providers + two in test_user_providers_model_switch, the latter caught by CI slice 2); the picker-timeout change now always passes timeout explicitly (5.0 on the non-picker path), so the pinned shapes gain the key.
…tat caching test_churn_across_more_files_than_fit_in_one_argv (e65ff96) asserts all 1200 checked-out files read dirty before normalization. Whether git diff content-compares an entry (seeing the CRLF churn) or trusts the stat cache depends on racy-git detection: entries whose recorded stat is non-racy (mtime older than the index write) read CLEAN. On CI a 1200-file checkout straddles that boundary nondeterministically — observed 92/1200 and 661/1200 dirty on two unrelated PRs within minutes (runs 30738759530, 30738842393). Empirically reproduced: freezing a non-racy stat cache gives 0/N dirty; bumping worktree mtimes past the index write forces content comparison and gives N/N deterministically. Fix: bump every worktree mtime after checkout in _managed_repo so all entries are stat-stale. Affects only the fixture; the production _normalize_managed_eol path is untouched.
Windows editors often save .env with a leading BOM; plain utf-8 left U+FEFF on the first key so multiplex get_secret missed that credential.
save_env_value writes values containing " or \ as escaped double-quoted strings, and every other .env reader in the repo (load_env/_parse_env_value, python-dotenv) reverses those escapes. load_env_file — the parser behind build_profile_secret_scope, which wraps every cron job and every multiplexed gateway turn — only stripped the outer quotes, leaving the escapes literal. A credential containing a double quote or backslash (JSON service-account blobs, generated secrets) authenticates fine interactively but 401s under scoped resolution, with no error pointing at the cause. Parse values with the canonical _parse_env_value so all readers agree byte-for-byte.
Unquoted values truncate only at a # preceded by whitespace (so KEY=foo#bar stays intact); quoted values scan escape-aware for the matching close quote, keep through it, and drop a trailing '# ...' remainder. Verified empirically against python-dotenv 1.2.2 on a 10-case corpus (full parity). Supersedes the approach in NousResearch#57718, whose scanner corrupted foo#bar-style values.
The adapter's __init__ and send_weixin_direct read WEIXIN_ACCOUNT_ID/ TOKEN/BASE_URL/CDN_BASE_URL via bare get_secret, which raises UnscopedSecretError when the DEFAULT profile's adapter constructs or sends unscoped under multiplexing (corrects the direction of NousResearch#66073 / NousResearch#68854, which tried to solve this by borrowing os.environ on every read — a cross-profile leak). Add a module-level _wx_secret helper following the established Slack SLACK_APP_TOKEN pattern (NousResearch#59739) and WhatsApp's _get_wsecret: a SCOPED miss returns the default (the scope is authoritative — no environ borrow), while an UNSCOPED read under multiplex falls back to os.environ, which is the default profile's own value. Regression tests cover both directions: scoped construction reads the scope's value and a scoped miss yields empty (no borrow); unscoped construction falls back to os.environ instead of raising.
…ment env Fixes NousResearch#69379 (v2026.7.20 Docker multiplex regression: the scoped runner reload dropped API_SERVER_* set via the container environment, silently losing the api_server platform) by the canonical mechanism — corrects the direction of NousResearch#69524, which patched gateway/config._getenv to fall through to os.environ on EVERY scoped miss, re-opening the cross-profile borrow for all credentials. API_SERVER_ENABLED / API_SERVER_HOST / API_SERVER_PORT / API_SERVER_CORS_ORIGINS are deployment listener settings (Docker compose environment: block, systemd Environment=), not profile secrets: they join _GLOBAL_ENV_EXACT so get_secret reads them from os.environ regardless of scope. API_SERVER_KEY is deliberately NOT allowlisted — it IS a credential and stays profile-scoped, which keeps tests/gateway/test_config.py's secondary-profile isolation semantics intact (a secondary profile without the key still doesn't bind a listener). Ports NousResearch#69524's regression test in the corrected form: container-env API_SERVER_* stays visible during the scoped runner reload while the key resolves through the profile scope; plus unit tests locking the allowlist membership and the deliberate exclusion of API_SERVER_KEY.
…verdict Salvaged from NousResearch#56982 (@rayjun): the live piece of the PR. The hermes_cli/config.py get_env_value scope-honoring change and its test_env_load_cache.py tests were already merged via ed1170c (NousResearch#76462) and are dropped here. tools/xai_http.py::get_env_value wrapped the scope-aware hermes_cli.config.get_env_value in except Exception + a raw os.environ fallback — swallowing UnscopedSecretError and borrowing the process env, so a multiplexed xAI credential read could silently pick up another profile's XAI_API_KEY. Narrow the except to ImportError (the only legitimate degraded case) so get_secret's verdict propagates: an unscoped multiplexed read fails closed, and a scoped miss returns the default instead of the foreign environ value. Co-authored-by: rayjun <rayjun0412@gmail.com>
…s, Mattermost Route IRC_SERVER_PASSWORD/IRC_NICKSERV_PASSWORD, LINE_CHANNEL_ACCESS_TOKEN/ LINE_CHANNEL_SECRET, TEAMS_GRAPH_ACCESS_TOKEN/TEAMS_CLIENT_SECRET and MATTERMOST_TOKEN reads at __init__/availability/standalone-send time through a module-level _get_scoped_secret helper (get_secret, UnscopedSecretError -> os.getenv fallback), mirroring whatsapp_common._get_wsecret / Slack NousResearch#59739. Scoped miss returns the default — no cross-profile environ borrow.
…stant, SMS, DingTalk NTFY_TOKEN, HASS_TOKEN, TWILIO_ACCOUNT_SID/TWILIO_AUTH_TOKEN and DINGTALK_CLIENT_SECRET now read through _get_scoped_secret. Also replaces the SMS adapter's bare os.environ["TWILIO_AUTH_TOKEN"]/["TWILIO_ACCOUNT_SID"] __init__ reads (KeyError-prone) with helper reads defaulting to "".
…Photon, Buzz FEISHU_APP_SECRET/FEISHU_ENCRYPT_KEY/FEISHU_VERIFICATION_TOKEN, WECOM_SECRET, PHOTON_PROJECT_SECRET/PHOTON_SIDECAR_TOKEN (adapter + auth.load_project_credentials) and BUZZ_PRIVATE_KEY now read through _get_scoped_secret.
… add parametrized regression tests api_server's __init__ API_SERVER_KEY read now matches the scoped _expected_api_key path. tests/gateway/test_adapter_startup_secret_scope.py asserts, for every migrated module: helper exists, scoped read wins, scoped miss returns default (no environ borrow), unscoped-under-multiplex falls back to environ without raising, and legacy single-profile reads still work.
The QQ adapter read QQ_APP_ID, QQ_CLIENT_SECRET, the QQ_STT_* backend config and the QQ_ALLOW_ALL_USERS policy flag through raw os.getenv, bypassing the active profile secret scope. In multiplex mode a secondary profile whose secret lives in its own .env (installed as an isolated scope, not into os.environ) would silently fall back to the default/primary profile's value — the same cross-profile collision fixed for the WeChat/weixin adapter in NousResearch#59662. Route these reads through a scope-aware resolver that reads the profile scope when one is installed (secondary profiles and per-turn inbound) and falls back to os.environ otherwise. The fallback is deliberate: the primary/active profile is constructed without a scope and owns os.environ, so a bare get_secret would raise UnscopedSecretError and break its startup. Mirrors gateway.config._getenv. Adds regression tests including active-profile-no-scope construction (the fail-closed case), plus scope-wins-over-environ, two-profile isolation, single-profile fallback, explicit-config precedence and STT key scoping.
…eads Review follow-up: the adapter-level resolver alone left three paths reading per-profile QQ_* values from raw os.getenv, so a secondary multiplex profile's scoped opt-in or credentials were ignored (or the primary's environ values leaked in): - gateway/authz_mixin.py: route the per-platform allow-all flag and the per-platform/group allowlist + allow-bots reads through the scope-aware gateway.config._getenv. Deployment-global GATEWAY_* reads intentionally stay on os.getenv. This makes the same fix effective for every own-policy platform, not just QQ; unscoped behavior is byte-identical to os.getenv. - gateway/run.py (_own_policy_open_startup_violation): resolve the per-platform dm/group policy and allow-all opt-in via _getenv; the secondary-profile caller already runs inside _profile_runtime_scope. - tools/send_message_tool.py (_send_qqbot): the QQ_APP_ID / QQ_CLIENT_SECRET fallbacks now honor the active profile scope. Tests: tests/gateway/test_qqbot_scope_paths.py covers all three paths end-to-end (scope wins, no environ inheritance for non-opted profiles, single-profile environ fallback unchanged); the STT suite now asserts QQ_STT_BASE_URL and QQ_STT_MODEL scoping alongside the API key. All five scoped-behavior tests fail on the previous commit and pass here.
Follow-up to the review-hardening commit: the existing cases exercised QQ_ALLOW_ALL_USERS but not the QQ_ALLOWED_USERS read at authz_mixin.py line 459, so a revert of that line to raw os.getenv would still pass. Add a scoped-allowlist DM case (scope admits the sender, environ does not) plus its isolation counterpart (a secondary scope listing a different user must not inherit the primary's environ allowlist). Both fail if line 459 reverts to os.getenv.
…ixin gate PR The cherry-picked NousResearch#60420 hunks that converted gateway/authz_mixin.py are dropped here: main's _auth_env/_platform_gate_env supersede them, and the remaining authz_mixin raw-read conversions (allow-all flag + allowlists at L459/501/879-885) land in a separate PR. Until that PR flips the allow-all read to scope-authoritative semantics, the cross-profile environ-opt-in inheritance case is a known gap — pin it as strict xfail so the separate PR flips it green.
Review follow-up on the NousResearch#62871 salvage (simplify pass, HIGH): 1. Ops unresolved at the wait deadline were RETAINED in the pending set. A permanently failing status endpoint (auth error, endless 500s, or a server that loses ops without 404) would grow the set forever and make EVERY later prefetch burn the full 10s budget re-polling it — and prefetch()'s bounded 3s join sits on the reply path, so that money-quote 'adds no response latency' claim breaks. Timed-out ops are now dropped (identical degradation to prefetch_waits_for_retain=False: possibly stale recall) with a WARNING so persistent server trouble is visible. Guard test mutation-checked (fails with eviction disabled). 2. Status polls now spaced 0.5s (was 0.05s shared with the local drain poll): a wedged op cost up to ~200 get_operation_status round trips per prefetch; now ~20 max over the default 10s budget.
…supermemory/mem0/retaindb) Route HINDSIGHT_API_KEY/HINDSIGHT_LLM_API_KEY, HONCHO_API_KEY, SUPERMEMORY_API_KEY, MEM0_API_KEY and RETAINDB_API_KEY reads through agent.secret_scope.get_secret so multiplexed turns resolve the active profile's key instead of the process environment. Also guard supermemory post_setup's os.environ write on not is_multiplex_active() — writing one profile's key into the process-global env pollutes sibling profiles; the single-profile convenience path is unchanged.
…ra/krea) OPENAI_API_KEY, DEEPINFRA_API_KEY and KREA_API_KEY now resolve via get_secret; the OpenAI client is constructed with the scoped key explicitly instead of relying on the SDK's implicit environ read.
…serbase/firecrawl)
…a models, FAL/XAI/VERCEL/DAYTONA/GITHUB presence, HERMES_API_KEY display) NOTION_API_KEY/LINEAR_API_KEY defaults, DEEPINFRA_API_KEY model gate, FAL_KEY via the file's own _scoped_credential helper, XAI/VERCEL/DAYTONA presence checks, GITHUB_TOKEN/GH_TOKEN + GitHub App creds in tirith/skills_hub, and the masked HERMES_API_KEY display in tui_gateway config.show all route through get_secret.
…awn time The detached meet_bot child inherits the process environment, not the parent's contextvar secret scope, so process_manager.start() now resolves HERMES_MEET_REALTIME_KEY/OPENAI_API_KEY through get_secret in the parent and passes it explicitly in the child env (spawn-wrap shape). Adds the consolidated per-family regression test file.
…coverage Add config-driven glibc malloc_trim for long-lived Hermes processes: - hermes_cli/mem_trim.py: trim_memory() with configurable cooldown, RSS snapshot telemetry, and forced-trim INFO logging - gateway/run.py: periodic trim in gateway housekeeping loop - tui_gateway/server.py: trim in idle reaper (~every 5 min) - tui_gateway/slash_worker.py: trim on turn boundary - run_agent.py: force trim on agent close - hermes_cli/config.py: context.memory_trim config section (enabled, cooldown_seconds, log_every_n, info_log_min_delta_mb) CSA tier-4 reviewed (4 rounds, 0 HIGH/MEDIUM/CRITICAL remaining). Supersedes PR NousResearch#63708 + NousResearch#64591 with enhanced telemetry and gateway/slash_worker coverage.
…test Simplify-pass follow-up on the NousResearch#66355 salvage: 1. _config_settings runs on EVERY trim attempt (before the cooldown check) and only reads — swap load_config for load_config_readonly. Deep-copying the whole config per attempt generates exactly the allocator garbage this module exists to release. Tests re-seamed. 2. Trim-failure logs demoted warning->debug at all 3 periodic sites (gateway housekeeping, idle reaper, slash worker): sibling failure branches in the same loops log at debug, and a persistent failure (e.g. broken import after a partial update) would otherwise warn every 60s forever. 3. The frame-inspection test now asserts the expected locals exist before reading them — a rename in _run_prompt_submit fails the test loudly instead of vacuously passing on None.
Efficiency-pass follow-up on the NousResearch#66355 salvage: force=True bypassed the cooldown entirely, and AIAgent.close() fires a forced trim for EVERY in-process child subagent close (delegate_tool child.close(), parent close step 5). A delegate batch of N children closing back-to-back in the gateway process stacked N+1 uncooled full gc.collect()+malloc_trim passes (50-500ms each with a large live heap). Forced trims now honor a 5s floor — bursts coalesce, the parent's final close-trim still fires. Guard test mutation-checked (floor zeroed -> test fails).
When read_file or search hits a non-existent path, ShellFileOperations
spawns a subprocess to stat the path and another to walk the parent
directory for "did you mean..." suggestions. A typo'd path retried 13
times (observed in the wild) costs 26 subprocess invocations + 13 ls
walks for a result we already know.
Add a per-task negative-result cache keyed by (op, resolved_path) with
a 60s TTL and a hard cap of 500 entries. On hit, return the cached
error JSON immediately and skip the subprocess + suggestion walk.
The cache is namespaced by operation ("read" vs "search") because the
two callers return different error JSON shapes ("File not found:" vs
"Path not found:"). Eviction:
* TTL (60s) — short, so a path that appears later isn't masked.
* write_file / patch on the same path — _invalidate_dedup_for_path
now also drops the negative-cache entry so a freshly-written file
is read from disk on the next call instead of returning a stale
"not found" stub.
Tests in tests/tools/test_file_tools.py cover:
* read cache hit skips the subprocess on retry
* cache is per-task (no cross-task pollution)
* successful reads do not poison the cache
* search cache hit skips the subprocess on retry
* read and search caches are namespaced (different error shapes)
* write_file invalidates the read negative cache
* TTL expiry evicts stale entries
Review follow-ups on the NousResearch#25387 salvage: 1. CRITICAL: a cached miss survived out-of-band file creation (terminal command, external process) for the full 60s TTL — breaking the common agent pattern 'check for file -> create it -> read it' (live-repro'd). Serve-side existence guard: one ~free stat before serving a cached miss; if the path now exists the entry is evicted and the real read runs. Also fixes the search-root variant (write under a cached-missing directory). Both mutation-checked. 2. notify_other_tool_call now clears the task's not_found entries too (belt: the dispatcher calls it for every non-read tool). 3. Tracking parity: the record sites no longer early-return. On upstream, error results flow through consecutive-loop detection and dedup bookkeeping; short-circuiting skipped that and broke TestDedupInvalidationTaskResolution when preceded by TestSilentFileMisplacementE2E (bisected: the early return at the read record site was the trigger). Recording is now side-effect-identical to upstream; serving from the cache remains the optimization. Also reuse the already-computed _resolved instead of resolving a second time.
Simplify-pass advisory on the NousResearch#25387 salvage: _read_tracker_lock is one global lock guarding every task's read/search bookkeeping (15 sites). A hung stat on a dead network mount inside it would stall all of them. Check-then-recheck matches the dedup mtime pattern 30 lines below: read the entry under the lock, stat outside, reacquire only to evict.
- Short-circuit the candidate waterfall on HTTP 401/403: an auth wall proves the endpoint family exists, so probing the alternate URL just doubles the wasted wait (the reported endpoint takes ~10s to return 401 without a key). - Stream the probe so 4xx never downloads a slow error body; responses are closed on every exit path. - Regression tests: single-call assertion on 401/403 (fails on main), negative-cache reuse, 404 waterfall preserved, no .json() on 4xx. Fixes NousResearch#69905 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-fixitfoundry chore: contributor email mapping for FixItFoundry
/api/status is the desktop's boot liveness probe (polled ~1/s) but since NousResearch#60537 every call ran a full topology scan — per-profile yaml.safe_load (pure-Python loader), psutil process probes, realpath walks — in the default executor. On multi-profile installs concurrent polls pile up and hold the GIL 14-16s, starving the event loop: the WS sidecar cannot flush gateway.ready, the desktop times out into the next stall, and boot escalates to the 'Hermes couldn't start' overlay (NousResearch#60800). Memoize the scan behind a 10s TTL with a collapse lock so concurrent polls share one scan. Topology only changes on gateway start/stop, so a <=10s stale badge is an acceptable trade for not starving the loop. The cache also keys on the collector's identity: tests monkeypatch _collect_profile_gateway_topology per case, and the identity check keeps them hermetic (a swapped collector is a miss) without a reset hook. py-spy captures during a failing boot land in _profile_platform_ports -> yaml.safe_load on executor threads (7 profiles, Windows). After: one cold-start scan, zero recurring stalls, desktop boots.
_handle_cron_fire verified the NAS-minted fire JWT by calling the fire-verifier inline on the event loop. That verifier resolves the NAS signing key from a JWKS URL — a synchronous HTTP GET on a cache miss (a cold PyJWKClient, or a rotated kid the cached client doesn't know) — so a slow or rate-limited portal stalls the whole event loop and starves every other adapter sharing it. NousResearch#64641 already documented this exact symptom (relay 504s on high-job-count instances) and cut the fetch frequency by caching the client per URL, but the residual cache-miss fetch still ran inline on the loop. Dispatch the verifier the same way the platform HTTP event verifier was hardened: await a coroutine verifier directly, run a sync one via asyncio.to_thread so its blocking I/O stays off the loop, and fail closed (reject with 401, never admit the fire) if the verifier raises — this is the only inbound that can trigger remote job execution. The verifier's JWK-client cache is already thread-safe (threading.Lock), so moving the call to a worker thread is safe. Adds regression tests: a sync verifier runs on a worker thread rather than the loop thread, a crashing verifier yields 401 with no fire, and a coroutine verifier is awaited.
…email chore: add contributor email mapping for wayne1992127
Replace f-string interpolation in logger calls with lazy %-style formatting across 10 files (38 instances). This follows Python logging best practices — the message is only formatted if the log level is enabled, avoiding unnecessary string concatenation overhead. Files changed: - trajectory_compressor.py (6) - mini_swe_runner.py (2) - agent/tool_executor.py (1) - agent/model_metadata.py (1) - agent/agent_runtime_helpers.py (3) - agent/chat_completion_helpers.py (3) - agent/conversation_loop.py (8) - tools/skills_hub.py (2) - tools/environments/docker.py (10) - gateway/kanban_watchers.py (2)
Re-applied from NousResearch#50508 onto current main: the executor moved from tools/ to agent/ and the JSON-error site was refactored away into _parse_tool_arguments, but these 8 f-string debug calls survived the move verbatim. Lazy %s formatting skips string interpolation when DEBUG is off — the Tool-result line interpolates the FULL tool output (can be 100KB+) on every tool call otherwise.
stop_profile_gateway() only killed the single PID recorded in the pid file. On repeated restarts, the new process overwrites the pid file before the old one exits, making older gateway instances invisible to subsequent stops. Each restart stacked another orphan — observed as N threads and N replies per Discord message. After killing the recorded PID, also call _reap_unsupervised_gateway_orphans() to sweep any remaining gateway processes for this profile. The reap function already handles the no-systemd guard and SIGTERM+SIGKILL escalation. Fixes NousResearch#75936
- Pass extra_exclude={pid} to _reap_unsupervised_gateway_orphans so the
killed PID isn't double-killed during the sweep (NousResearch#75936).
- Add extra_exclude param to _reap_unsupervised_gateway_orphans signature.
- Replace bare except:pass with logger.debug for diagnosability.
- Fix existing test (mock _reap_unsupervised_gateway_orphans so it
doesn't scan real processes and trigger conftest live-system guard).
- Add regression test asserting the killed PID is excluded from the sweep.
pcm_to_wav staged every captured utterance in a NamedTemporaryFile just to hand ffmpeg an input path, then unlinked it. Feed the PCM to ffmpeg's stdin instead: one fewer file created, written, read back and removed per voice utterance, and the try/finally cleanup goes away with it. The WAV output deliberately still goes to output_path rather than being captured from stdout. ffmpeg cannot seek on a pipe, so a piped WAV is written with placeholder 0xFFFFFFFF RIFF/data chunk sizes -- Python's wave module then reports 2147483647 frames for a 1s clip, and strict readers misjudge the length. Writing to the real path lets ffmpeg seek back and patch the header. Tests cover both halves: that the PCM goes over stdin with no temp file, and (when ffmpeg is installed) that the resulting header reports the true frame count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Simplify-pass follow-up on the NousResearch#68157 salvage (regression-neutral \u2014 the old temp-file version was equally bare): capture ffmpeg's -loglevel error output so a CalledProcessError carries the real message, and log it at the voice-input catch site. Parity with transcription_tools' ffmpeg call sites. Live-verified: forced ffmpeg failure produces the captured message in the exception.
The WSL voice-detection tests assert that a no-forwarding WSL environment (no PULSE_SERVER, no PIPEWIRE_REMOTE) hard-blocks voice mode. But the WSL2 PowerShell TTS fallback (added later) relaxes that block when powershell.exe + ffmpeg exist on the host: detect_audio_environment() downgrades to a notice and reports available=True. The tests never pinned _wsl_powershell_tts_available, so their pass/fail depended on the host environment — a WSL machine with powershell.exe and ffmpeg (both present by default in WSL2) failed test_wsl_without_forwarding_ still_blocks and test_wsl_without_pulse_blocks_voice. Deterministic unit tests must not depend on host binaries. Pin the fallback to False in the no-forwarding tests (they test the hard-block path) and add an explicit test for the fallback path (available True with a WSL notice) so that behavior is covered deterministically.
Owner
Author
|
Closing - base diff computed against stale fork main; will recreate correctly. |
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
20 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.
What does this PR do?
Pin the WSL2 PowerShell TTS fallback in the WSL voice-detection tests so they pass/fail deterministically on any host. The no-forwarding tests previously depended on whether powershell.exe + ffmpeg exist on the machine (a WSL2 host with both flipped them to fail).
Related Issue
No direct issue — discovered via code review and reproduced live (see below).
Related PRs reviewed during the duplicate check (none covers this change):
Changes Made
fix/wsl-voice-test-determinism— 2 file(s) changed vs base:tests/tools/test_voice_mode.pytests/tools/test_voice_wsl_pipewire.pyIn tests/tools/test_voice_wsl_pipewire.py, the _base fixture now pins _wsl_powershell_tts_available to False (the no-forwarding tests exercise the hard-block path); a new test covers the fallback path (available True with a WSL notice). In tests/tools/test_voice_mode.py, test_wsl_without_pulse_blocks_voice pins the same fallback to False.
How to Test
Validation completed (recorded by prp):
<full suite>: branch : 24415 passed, 9 failed vs baseline : 24413 passed, 10 failed — 2 branch-only failure(s).Logs
Sabotage verification: