Skip to content

fix(speech): pin closure error types + Manager import for MSVC compile - #18

Merged
mastwet merged 4 commits into
mainfrom
fix/windows-speech-shell
Oct 4, 2026
Merged

mastwet merged 4 commits into
mainfrom
fix/windows-speech-shell

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Fixes the windows-shell-check failures surfaced on the merged DN-8C CI job — the agent-diva-shell lib (src/speech/) could not be compile-checked on the authoring VM (no webkit2gtk), so these are the errors only MSVC/CI could see:

  • commands.rs: bare Ok(..) inside (|| { .. })() closures left Result<_, E> ambiguous — InvokeError has multiple From impls (tauri::Error, T: Serialize, blanket From<T> for T), so E0282/E0283 fired at 6 sites. Each closure now declares -> SpeechResult<T> so .map_err(InvokeError::from) resolves to From<SpeechError> exactly like speech_transcribe's already-compiling path.
  • voice_asset_delete: assets().delete() returns SpeechResult<&'static str> but the command promised Result<String, InvokeError> (E0308) — now .map(String::from).
  • mod.rs: speech_dir calls app.path() without tauri::Manager in scope (E0599) — imported; the now-unused Manager in commands.rs dropped (kills the warning too).

No behavior change; error surface unchanged (SpeechError → InvokeError mapping identical). Verification is windows-shell-check on this branch — the crate still cannot compile on Linux.

Sibling context: agent-vivy#29 gofmt drift fixed pre-merge (8681933d); no other compile errors existed in the failing log beyond these files.

Link to Devin session: https://app.devin.ai/sessions/c016e7bc2d98433e93f645345cb937e5
Open in Devin Desktop: https://app.devin.ai/desktop/session/c016e7bc2d98433e93f645345cb937e5?variant=devin
Requested by: @mastwet

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

windows-shell-check (first Windows compile) surfaced errors the Linux
VM cannot build: bare `Ok(..)` inside command closures left E
uninferred against InvokeError's multiple From impls (E0282/E0283 at
6 sites), voice_asset_delete returned &'static str against String
(E0308), and speech_dir called AppHandle::path() without
tauri::Manager in scope (E0599). Closures now declare
-> SpeechResult<T> so map_err(InvokeError::from) resolves; delete
maps to String; Manager imported in speech/mod.rs (and dropped from
commands.rs where it was unused). Verification: deferred to
windows-shell-check on this branch — the shell crate still cannot
compile on Linux (webkit2gtk).

Co-Authored-By: 大湿 <1136523089@qq.com>
windows-shell-check round 2: fake-artifact tests spawn `cl`, which is
not on PATH on windows-latest without the MSVC dev environment — add
ilammy/msvc-dev-cmd before the test step. load_missing_abi_header wrote
a literal vivy-shared.so fixture, but VivyLibrary::load resolves the
platform name (vivy-shared.dll on Windows) before the header check, so
the missing-abi branch was never reached — write artifact_name() with
the same per-OS names as library_path().

Verified: cargo test -p vivy-bridge 7/7 on Linux (real-artifact env
present).

Co-Authored-By: 大湿 <1136523089@qq.com>
fake_vivy.c used GCC-only __attribute__((visibility)) — cl rejects it
(error goes to stdout, which the assert did not print). Add VIVY_EXPORT
mapping to __declspec(dllexport) on _MSC_VER; also include stdout in
the build-failure message. Verified: cargo test -p vivy-bridge 7/7 on
Linux.

Co-Authored-By: 大湿 <1136523089@qq.com>
@devin-ai-integration
devin-ai-integration Bot force-pushed the fix/windows-speech-shell branch from 8a2a842 to 0e2da20 Compare October 4, 2026 12:03
main already imports Manager at the top of speech/mod.rs; the rebased
import landed a second copy (E0252).

Co-Authored-By: 大湿 <1136523089@qq.com>
@mastwet
mastwet merged commit d81381f into main Oct 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant