Skip to content

fix(desktop): preserve failed voice recordings - #72976

Open
DrMaks22 wants to merge 1 commit into
NousResearch:mainfrom
DrMaks22:codex/webui-stt-recovery
Open

fix(desktop): preserve failed voice recordings#72976
DrMaks22 wants to merge 1 commit into
NousResearch:mainfrom
DrMaks22:codex/webui-stt-recovery

Conversation

@DrMaks22

@DrMaks22 DrMaks22 commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Desktop currently deletes its uploaded recording in finally, so a provider error, timeout, cancellation, or malformed response can destroy a long recording before the user has any recovery path. The raw provider exception can also cross the API boundary and expose credential-bearing details.

This change makes Desktop STT failure-safe:

  • stage the original upload atomically in a private, profile-scoped cache before transcription starts;
  • retain it only on failure/interruption, with configurable TTL, entry, and byte caps (24h / 50 / 500 MiB by default; hard-capped at 7d / 500 / 2048 MiB);
  • coordinate concurrent server/CLI processes with root locks, per-attempt OS leases, CAS attempt transitions, and durable cleanup_pending tombstones;
  • expose hermes stt recovery list|retry|save|discard for explicit recovery;
  • return only structured, opaque recovery metadata to Desktop and redact provider errors, filesystem paths, and credential material;
  • give the Desktop error toast a localized Copy recovery command action, including the correct profile for local pooled or remote backends;
  • keep recovery audio out of profile clone/export and backup flows;
  • persist each tile's authoritative profile owner so recovery actions remain correctly scoped after bounded session metadata is evicted.

Related: #37889, #37890, #53488, #66626.

Maintainer review follow-up

The latest revision addresses both automated review comments:

  • established no-speech results still return 200 with an empty transcript and remove the staged recording;
  • async profile resolution patches the captured origin tile, not whichever profile bucket becomes active later.

The same review exposed adjacent equal-ID/profile races. The final revision therefore carries the authoritative owner through tab focus, native-window IPC/URL routing, resume/retry, branch/archive/delete, and live-runtime binding. Async resume completion uses an owner-aware compare-and-set, and the native-window registry uses an unambiguous tuple key. These paths have adversarial duplicate-ID, profile-switch, stale-completion, URL-parser, and delimiter-collision tests.

Lifecycle and privacy

The cache lives under <HERMES_HOME>/.cache/stt-recovery/. POSIX directories/files are created as 0700/0600; Windows uses the account's inherited ACLs. Manifests contain no transcript, raw exception, absolute path, or credential material. Expired entries are hidden immediately and pruned on the next cache access; there is no background service while Hermes is stopped.

Recovery is enabled by default to prevent silent data loss. Operators can disable it or reduce its limits through stt.recovery. This is an explicit privacy/reliability trade-off: physical expiry is access-triggered, while confidentiality is constrained by owner-only storage, opaque IDs, bounded retention/capacity, export/backup exclusion, and no remote list/read endpoint.

A validated successful transcript commits deletion before the response is returned. This PR intentionally does not add a renderer acknowledgement protocol, so the narrow case where the connection drops after successful STT but before Desktop inserts the returned transcript remains documented follow-up scope.

Validation

  • selected Python recovery/web/profile/backup suite: 345 passed;
  • complete Desktop UI suite: 355 files, 3153 tests passed;
  • complete Electron suite: 74 files passed, 1 skipped; 870 tests passed, 2 skipped;
  • Desktop TypeScript typecheck, strict ESLint, and Prettier checks passed;
  • Ruff, ty, git diff --check, and the Windows-path production scan passed;
  • full diff security review: all 44/44 source-like worklist rows and 6/6 candidates closed; no reportable or deferred security findings.

Scope

This protects recordings submitted through Desktop/WebUI /api/audio/transcribe. Inbound voice messages handled by messaging-platform adapters use a separate pipeline and are unchanged.

@DrMaks22
DrMaks22 marked this pull request as ready for review July 27, 2026 23:31
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard tool/tts Text-to-speech and transcription area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 27, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tackling a real data-loss and error-redaction problem. Current main still unlinks the upload in finally and can return raw provider exception text from hermes_cli/web_server.py:4256-4264.

Problems

  • hermes_cli/web_server.py:4568 makes the established no-speech result a provider_error response. The retained test at tests/hermes_cli/test_web_server.py:3072-3103 supplies success=False plus an empty-transcript error and requires 200 with an empty transcript; this branch returns 400 and retains the recording instead.
  • apps/desktop/src/app/contrib/hooks/use-session-tile-delegate.ts:75-83 awaits profile resolution before calling patchSessionTile. That helper persists only the currently active profile bucket (apps/desktop/src/store/session-states.ts:408-432), so a rail-profile switch during the await leaves the original tile without its owner backfill.

Suggested changes

  • Preserve the current empty-transcript path before provider_error, cleaning up any staged recovery record rather than surfacing a failure.
  • Persist the owner against the tile's originating bucket or guard the async completion against profile changes; cover the switch-during-resolution case.

This is an automated hermes-sweeper review.

Comment thread hermes_cli/web_server.py
Comment thread apps/desktop/src/app/contrib/hooks/use-session-tile-delegate.ts Outdated
@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@DrMaks22
DrMaks22 force-pushed the codex/webui-stt-recovery branch from 4674e94 to 8a0e031 Compare July 30, 2026 20:19
@DrMaks22

Copy link
Copy Markdown
Author

Updated to 8a0e031007 on current main. Both review findings are addressed, their regression tests are included, and both threads are resolved. Local validation is green (345 selected Python tests; 3153 UI tests; 870 Electron tests; typecheck/lint/Ruff/ty/Windows checks). GitHub is currently holding the fork CI run as action_required: CI run 30578656828. A repository maintainer needs to choose Approve and run workflows; that should allow the required All required checks pass gate to report.

@andrexibiza

Copy link
Copy Markdown
Contributor

Vox Lockin lane 09 verification receipt:

  • Not mergeable as-is: git merge-tree against current main (70db671) conflicts in 4 files (session-tile.tsx, session-row.tsx, directive-text.tsx, notifications.test.ts) — main has diverged substantially on the tile/profile plumbing this PR rewrites.
  • Desktop-class assessment: the core premise (preserve failed voice recordings server-side; profile-scoped transcribeAudio) is largely hermes_cli/web_server.py + tile/profile refactoring, which is outside the desktop voice-hook lane and overlaps lanes 02/07 territory. The desktop hook class (use-voice-conversation / recorder / playback) is not affected by the conflicts.
  • No desktop-hook gap found; the desktop voice conversation error path already surfaces transcription failures via toast + re-listen. Recommend rebasing the server-side portion for lane 02/07 review; no duplicate work opened from lane 09.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants