fix(secrets): collect persona-level TTS provider SecretRefs#89636
fix(secrets): collect persona-level TTS provider SecretRefs#89636SebTardif wants to merge 3 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 2:21 PM ET / 18:21 UTC. Summary PR surface: Source +74, Tests +175, Docs +16. Total +265 across 12 files. Reproducibility: yes. in source: the linked report provides a concrete file-SecretRef persona configuration, and the proposed collector/registry additions directly cover its previously omitted paths. A full current-head provider conversion was not demonstrated in the supplied evidence. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the common collector and registry approach, then add redacted current-head proof that a persona-level file SecretRef reaches a real TTS conversion path and produces the expected provider outcome. Do we have a high-confidence way to reproduce the issue? Yes in source: the linked report provides a concrete file-SecretRef persona configuration, and the proposed collector/registry additions directly cover its previously omitted paths. A full current-head provider conversion was not demonstrated in the supplied evidence. Is this the best way to solve the issue? Yes, conditionally: extending the existing shared secret collector and target registry is the narrowest maintainable fix rather than adding TTS-plugin-specific resolution. The remaining need is end-to-end proof of that shared path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b7f99d0ffd2a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +74, Tests +175, Docs +16. Total +265 across 12 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (36 earlier review cycles; latest 8 shown)
|
631a4e8 to
9c158e5
Compare
|
@SebTardif thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
|
@clawsweeper re-review Addressed the P1 agent-runtime target gap: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Fixed providerIdPathSegmentIndex for agent persona TTS (6→7) and agent TTS providers (4→5), added registry assertions, and refreshed terminal proof of providerId=elevenlabs plus persona file SecretRef collection. Live ElevenLabs synthesis still not available here; #101204 remains the fail-open product decision. |
9d99019 to
2ff6193
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Rebased onto current main and addressed the integration findings:
Local proof (unit/registry, exact head after rebase): $ pnpm exec vitest run src/secrets/runtime-config-collectors-tts.test.ts src/secrets/target-registry.test.ts src/cli/command-secret-targets.test.ts
Test Files 3 passed (3)
Tests 57 passed (57)Still not available here: live Gateway TTS synthesis with a real provider credential. Collector and registry paths are covered by tests and the previous terminal probes. |
2ff6193 to
41d4bdc
Compare
|
@clawsweeper re-review Updated the PR body with a live file-source persona SecretRef path: isolated config + |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
17b6784 to
b49daf3
Compare
|
@clawsweeper re-review Refreshed onto current main with tts/agents.entries path schema. Re-ran secrets audit (clean) and infer tts convert (local resolve then ElevenLabs 401). Persona jsonPaths confirmed in collection set. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(secrets): collect persona-level TTS provider SecretRefs This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
Walk tts.personas.*.providers.*.apiKey (and agents.entries.*.tts.personas) when collecting runtime SecretRefs so persona overrides resolve like top-level TTS providers. Registers audit/configure targets and CLI secret-policy ids for the current tts / agents.entries path schema. Refreshed onto current main (paths no longer use messages.tts / agents.list). Signed-off-by: Sebastien Tardif <[email protected]>
Keep secretref-user-supplied-credentials-matrix.json in registry order so target-registry.docs tests stay green after persona SecretRef targets. Signed-off-by: Sebastien Tardif <[email protected]>
The persona-level TTS registry entry was unclassified, so the sampled class coverage test failed with tts.personas.*.providers.*.apiKey. Treat tts.personas.* the same class as tts.providers.*. Signed-off-by: Sebastien Tardif <[email protected]>
a044bc6 to
c8b15b5
Compare
|
@clawsweeper re-review Refreshed live proof on current head c8b15b5 (post parity-classify CI fix):
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
What Problem This Solves
Persona-level TTS provider API keys were not collected as SecretRefs, and agent persona registry paths bound the wrong path segment as
providerId(literalprovidersinstead ofelevenlabs). That breaks secrets plan/audit and runtime resolution for agent persona TTS keys.Fixes #89607
Summary
The secret collector walks top-level TTS provider API keys but did not walk persona-level provider keys under
tts.personas.*.providers.*.apiKeyoragents.entries.*.tts.personas.*.providers.*.apiKey. When a persona overrides the TTS provider config with a file-source SecretRef, the secret resolver never saw the ref, so theapiKeyreached the plugin as an unresolved object instead of the actual key string. The plugin then reportednot configured.This fix:
collectTtsApiKeyAssignmentstts.personas.*.providers.*.apiKeyandagents.entries.*.tts.personas.*.providers.*.apiKey)providerIdbinding so persona paths resolve to the provider key (e.g.elevenlabs)tts.personas.*in exec SecretRef id parity sampling (CI)Closes #89607
Evidence
Live OpenClaw CLI on branch head
c8b15b5f5f4with an isolated config that uses file-source SecretRefs for persona TTS keys.secrets auditreportsstatus: cleanwhen the secrets file is mode 600. With intentionally open secrets permissions, audit lists all three collected paths (provider, persona, agent-entry persona).infer tts convert --localresolves those SecretRefs locally and reaches the ElevenLabs HTTP API (401 invalid key with a dummy secret), which proves resolution instead of the pre-fixnot configuredfailure.Real behavior proof
Behavior addressed: Persona-level TTS provider SecretRefs under tts.personas and agents.entries.*.tts.personas must be collected, audited, and resolved so TTS conversion receives a string API key rather than an unresolved SecretRef object.
Real environment tested: macOS, OpenClaw branch fix/tts-persona-secretref at /tmp/oc-89636 commit c8b15b5 (includes parity classify fix; rebased on current main). Isolated state dir /tmp/oc-proof-state-89636 with file secrets provider (dummy key only; never a real credential).
Exact steps or command run after this patch:
Step 1. Configured tts.providers, tts.personas.narrator.providers, and agents.entries.main.tts.personas.narrator.providers with file SecretRefs (source file, provider default, id /elevenlabsNarratorKey) and a mode=json secrets file with mode 600.
Step 2. Audited secret collection and resolvability:
Step 3. Ran local TTS conversion so runtime resolution must supply the persona/provider key:
node scripts/run-node.mjs infer tts convert --local --text 'hello proof' --json --output /tmp/proof-89636-tts.wavEvidence after fix: terminal output from the patched CLI at current head c8b15b5.
Secrets audit clean with resolvability complete:
Path-collection diagnostic with intentionally open secrets file permissions (mode 644) lists all three collected paths as unresolved with the same permission error, proving persona jsonPaths enter the collection set:
TTS convert resolves SecretRefs locally then reaches ElevenLabs with a string key (dummy key only):
Observed result after fix: secrets audit is clean when secrets are mode 600. Persona paths are collected. Local convert resolves SecretRefs and reaches ElevenLabs authentication with the dummy key (401 Invalid API key), proving a string key was delivered rather than an unresolved object.
What was not tested: Live Gateway websocket secrets.resolve with a paired device token; production ElevenLabs synthesis with a real paid key.