fix(plugins): start configured speech providers#76540
Conversation
|
Codex review: needs changes before merge. Summary Reproducibility: yes. at source level: current main's startup planner never inspects Next step before merge Security Review findings
Review detailsBest possible solution: Align startup speech-provider discovery with effective TTS provider/config merge semantics, then land the narrow startup planner, regression-test, and changelog change for #76481. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main's startup planner never inspects Is this the best way to solve the issue? No, not quite. The PR is in the right layer, but active-persona provider startup should preserve root provider-block disablement unless the persona binding explicitly re-enables that provider. Full review comments:
Overall correctness: patch is correct Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 9772ce6ce975. |
|
Re: the P3 persona disablement finding — The concern is that The only practical effect of the current code in this edge case is starting an extra plugin that may go unused, which is strictly safer than the alternative (not starting a needed plugin — the original bug). The triggering config is also inherently contradictory: disable at root but configure in persona without re-enabling. Happy to add the three-state activation check as a follow-up if real-world configs hit this, but for this PR the over-inclusive startup behaviour is intentional. |
|
/clawsweeper automerge |
|
ClawSweeper 🐠 automerge status This pass ended as a no-op: no narrow repair surfaced, so ClawSweeper left the branch untouched. Executor outcome: no planned fix actions. Worker actions:
No push, rebase, replacement PR, merge, or ClawSweeper re-review happened on this pass. fish notes: model gpt-5.5, reasoning high. Automerge progress:
|
46cd698 to
623f45e
Compare
* fix(plugins): start configured speech providers * fix(plugins): mirror tts provider selection
* fix(plugins): start configured speech providers * fix(plugins): mirror tts provider selection
* fix(plugins): start configured speech providers * fix(plugins): mirror tts provider selection
* fix(plugins): start configured speech providers * fix(plugins): mirror tts provider selection
* fix(plugins): start configured speech providers * fix(plugins): mirror tts provider selection
Summary
messages.tts.provider=microsoftand still get no Telegram voice replies because the Microsoft speech plugin never started with the Gateway.edgealias for Microsoft.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
messages.ttsspeech-provider ownership as a startup activation signal.messages.tts.providerselecting a plugin-owned speech provider.Regression Test Plan (if applicable)
src/plugins/channel-plugin-ids.test.tsmessages.tts.provider: "microsoft", legacy"edge", and provider blocks such asmessages.tts.providers["tts-local-cli"]include the owning speech plugin in Gateway startup unless explicitly disabled.User-visible / Behavior Changes
Configured TTS speech providers such as Microsoft and Local CLI are now activated during Gateway startup, so voice replies work after selecting those providers.
Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: N/ARepro + Verification
Environment
messages.tts.provider=microsoft,plugins.entries.microsoft.enabled=trueSteps
messages.tts.provider=microsoft.Expected
Actual
Evidence
Before fix, local planner repro omitted Microsoft:
After fix, local planner repro includes Microsoft:
Validation run:
Human Verification (required)
messages.tts.provider=microsoft, includes Local CLI for provider-block config, maps legacyedgeto Microsoft, and respects disabled provider/plugin config.plugins.entries.microsoft.enabled=falsedo not force activation.Review Conversations
Compatibility / Migration
Risks and Mitigations