Skip to content

fix(media-understanding): append actionable install hint when media provider is missing#96790

Closed
wangmiao0668000666 wants to merge 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/media-provider-missing-hint
Closed

fix(media-understanding): append actionable install hint when media provider is missing#96790
wangmiao0668000666 wants to merge 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/media-provider-missing-hint

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

When a user has configured a media provider plugin that is not installed (e.g. groq after externalization), OpenClaw throws a bare Error: Media provider not available: groq. The user sees no actionable next step — no install command, no registry refresh hint, no doctor fix. They must search docs or Discord for "groq provider not available" to discover the fix.

This is a user experience regression introduced when Groq was externalized from a bundled to an installable plugin. Previously the provider was always available; now users who migrate their config get a dead-end error with no recovery path.

Beyond the user experience issue, there is a false positive risk: the old implementation checked providers[] in the catalog, which would emit a misleading install hint for amazon-bedrock — a provider that has a catalog entry with providers[] but does NOT own the mediaUnderstandingProviders contract. This PR fixes both issues.

Changes

New helper formatMissingProviderHint (~42 LoC) — gates on contracts.mediaUnderstandingProviders (not the generic providers[] catalog) and returns formatted hint for cataloged media providers, empty string for all others.

  • src/media-understanding/runner.entries.ts:698-744formatMissingProviderHint helper.
  • src/media-understanding/runner.entries.ts:689-691 — updated JSDoc from providers[] to contracts.mediaUnderstandingProviders.
  • src/media-understanding/runner.entries.ts:800 — 1 throw site modified. Audio and video capability-missing errors (:808, :887) intentionally NOT modified — they do NOT receive the install hint to avoid misleading operators about an already-loaded provider.
  • src/media-understanding/runner.entries.guards.test.ts — 8 inline tests covering all tiers and backward-compat.

Follows the pattern from the canonical issue #95658; supersedes the earlier attempt #95926 with a cleaner scope.

Design Rationale

Why a separate formatMissingProviderHint function? The hint logic requires catalog lookup, contract gating, and resolveOfficialExternalPluginRepairHint integration — concerns orthogonal to runProviderEntry's core dispatch loop. A dedicated function keeps the throw site readable and the hint logic independently testable.

Why contracts.mediaUnderstandingProviders instead of the generic providers[] catalog? The combined plugin/channel catalog contains entries that declare providers[] but do not own the media-understanding capability (e.g. Amazon Bedrock has a catalog entry with bedrock provider IDs but no contracts.mediaUnderstandingProviders). A providers[]-based gate would emit a misleading install hint pointing to the wrong package. The contract gate ensures the hint only fires for packages that actually own the missing capability.

Why intentionally skip the audio-transcription (:808) and video-capability (:887) throw sites? Those errors fire when a media provider IS loaded in the registry but lacks the requested capability (e.g. a vision model loaded for audio transcription). Appending an "install the provider" hint there would mislead operators into reinstalling a provider they already have — worse than the original error. The single modified site (:800) is the true "provider not found" path where the hint is accurate.

Why formatCliCommand wrapping? Ensures the install and doctor commands are formatted consistently with other OpenClaw CLI hint surfaces. The official-external-plugin-catalog and official-external-plugin-repair-hints modules are reused rather than reimplementing catalog traversal.

Real behavior proof

  • Behavior addressed: unbounded bare Media provider not available: groq error; now appends actionable install command, registry refresh, restart, and doctor fix instructions for providers with a mediaUnderstandingProviders catalog contract.
  • Real environment tested: Node v22.22.0, production runProviderEntry function imported from src/media-understanding/runner.entries.ts.
  • Exact steps or command run after this patch: node --import tsx _proof_groq_product.mts (working tree only, not committed)
  • Evidence after fix: 21/21 tests pass (12 existing + 9 new). runProviderEntry({providerId: "groq", providerRegistry: empty}) throws Media provider not available: groq with a 217-character install hint containing install, registry refresh, restart gateway, and doctor fix instructions. Non-MUP providers (amazon-bedrock, feishu, unknown ids) produce the unchanged legacy error.
  • Observed result after fix: runProviderEntry({providerId: "groq"}) throws with 217-character install hint containing install command, registry refresh, restart, and doctor fix instructions. Non-MUP providers (amazon-bedrock, feishu, unknown ids) produce the unchanged legacy error.
  • What was not tested: live gateway SESSRUN with a configured Groq media provider (the proof exercises the same runProviderEntry function on the same Node runtime that the gateway uses).

Out of scope

Risk checklist

  • User-visible behavior change? Yes — missing media provider error now appends install/refresh/restart/fix instructions for MUP-contract providers; all other providers and capability-missing errors produce the unchanged legacy message.
  • Config/env/migration change? No.
  • Security/auth/secrets/network/tool-execution change? No.
  • Highest-risk area: false positive install hint for a provider that is loaded but capability-missing. Mitigated by gating only on contracts.mediaUnderstandingProviders and intentionally skipping audio/video capability errors.

Diff stats

2 files changed, 121 insertions(+), 8 deletions(-)

AI-assisted: implemented and proof-tested with AI assistance; reviewed by a human before submission.

…rovider is missing

Add formatMissingProviderHint helper that produces install/doctor
hints for the official external provider catalog. Applies to 3 throw
sites: provider not available, audio transcription not available, and
video understanding not available.

Tier 1 (catalog-known): actionable install + registry refresh + gateway
restart + doctor fix commands.
Tier 2 (non-cataloged): empty string, legacy message preserved verbatim.
Tier 3 (empty/non-plugin-shaped): empty string, legacy message preserved.
Channel-only ids (feishu) return empty to avoid misleading hints.
@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 9:03 PM ET / 01:03 UTC.

Summary
The PR adds a media-understanding-contract-gated install/registry/doctor hint to the missing media-provider error and extends guard tests for Groq plus negative catalog cases.

PR surface: Source +55, Tests +58. Total +113 across 2 files.

Reproducibility: yes. at source level: current main and v2026.6.10 still throw the bare missing-provider error, and the canonical issue includes real CLI/Gateway Groq failure logs. I did not run a live packaged Gateway reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Recovery Throw Sites: 1 changed, 2 unchanged. Only the absent-provider branch receives install guidance, while loaded-provider audio/video capability errors keep their previous semantics.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/media-understanding/runner.entries.guards.test.ts, migration/backfill/repair: src/media-understanding/runner.entries.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95658
Summary: This PR is the active candidate fix for the open Groq/media-understanding missing-provider diagnostic issue; the older same-author PR was closed in favor of this cleaner version.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Have a maintainer decide whether the generic gateway restart wording should be tightened before merge.

Risk before merge

  • [P1] The hint tells operators to refresh the plugin registry and restart the gateway, but the canonical issue reporter needed a full systemd stop/start for the Telegram/Gateway path, so maintainers should confirm whether the generic restart wording is acceptable.
  • [P1] The contributor proof exercises production runProviderEntry, not a packaged Gateway or systemd service path, so the exact reported deployment flow remains unproven before merge.

Maintainer options:

  1. Accept The Current Diagnostic
    Maintainers can land the source-level fix as-is while owning the remaining wording uncertainty for systemd-managed gateways.
  2. Tighten Gateway Reload Wording
    Ask for a small pre-merge text tweak if the supported recovery should explicitly say full gateway stop/start instead of generic restart.
  3. Wait For Packaged Gateway Proof
    Pause until someone supplies redacted packaged Gateway or systemd proof that the displayed recovery steps match the reported environment.

Next step before merge

  • Maintainer review should decide the remaining gateway reload wording risk and then land the PR or request a small wording tweak.

Security
Cleared: The diff changes TypeScript error formatting and tests only; it adds no dependencies, workflows, lockfiles, scripts, secret handling, or package execution path.

Review details

Best possible solution:

Land the contract-gated missing-provider diagnostic once maintainers accept the generic gateway restart wording or request a small wording tweak.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main and v2026.6.10 still throw the bare missing-provider error, and the canonical issue includes real CLI/Gateway Groq failure logs. I did not run a live packaged Gateway reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, mostly: a mediaUnderstandingProviders-gated hint at the missing-provider branch is the narrow owner-boundary fix and avoids misleading loaded-provider capability errors. The remaining question is operator wording, not a clear code defect.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1bccd2930437.

Label changes

Label changes:

  • add status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Sufficient (terminal): The PR body and follow-up comments provide after-fix terminal proof through production runProviderEntry for Groq plus negative catalog cases, though not a live packaged Gateway run.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 🔁 re-review loop.

Label justifications:

  • P1: The PR targets a latest-release Groq voice transcription regression affecting a real media/channel workflow.
  • merge-risk: 🚨 other: The diff publishes operator recovery wording whose exact Gateway/systemd reload behavior is not proven by CI or the posted runtime proof.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 🔁 re-review loop: A fresh ClawSweeper review was explicitly requested after the latest review. Sufficient (terminal): The PR body and follow-up comments provide after-fix terminal proof through production runProviderEntry for Groq plus negative catalog cases, though not a live packaged Gateway run.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comments provide after-fix terminal proof through production runProviderEntry for Groq plus negative catalog cases, though not a live packaged Gateway run.
Evidence reviewed

PR surface:

Source +55, Tests +58. Total +113 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 62 7 +55
Tests 1 59 1 +58
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 121 8 +113

What I checked:

Likely related people:

  • ly-wang19: Local blame for the current missing-provider branch, media registry contract key, Groq catalog entry, and official catalog helper points to the grafted current-source snapshot commit; confidence is limited because the checkout history is shallow/grafted. (role: current-source snapshot contributor; confidence: low; commits: c1336b6b412b; files: src/media-understanding/runner.entries.ts, src/media-understanding/provider-registry.ts, scripts/lib/official-external-provider-catalog.json)
  • sunlit-deng: Authored the issue-cited merged external-plugin startup PR that made external plugin loading/restart behavior relevant to this Groq recovery path. (role: adjacent external-plugin startup contributor; confidence: medium; commits: aaab95b70af2; files: src/plugins/gateway-startup-plugin-ids.ts, src/plugins/channel-plugin-ids.test.ts)
  • vincentkoc: Merged the related external-plugin startup PR and authored follow-up commits in the same plugin startup/enablement area. (role: adjacent merger and follow-up contributor; confidence: medium; commits: 037224968c7f, 934dfd3c579a; files: src/plugins/gateway-startup-plugin-ids.ts, src/plugins/channel-plugin-ids.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 25, 2026
…ndingProviders contract

ClawSweeper P2: the previous gate checked any provider catalog entry with
a non-empty providers[] block, which could emit misleading install hints
for packages that don't own media-understanding providers (e.g. Amazon
Bedrock). Now gates on contracts.mediaUnderstandingProviders only.

- amazon-bedrock: was positive → now negative (no MUP contract) ✅
- groq: stays positive (has contracts.mediaUnderstandingProviders: ["groq"]) ✅
- Remove unused normalizeOptionalString import
- Add product-path proof through all 3 throw sites (8/8 assertions)

Co-Authored-By: Claude <[email protected]>
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed both findings:

[P2] Gate on mediaUnderstandingProviders contract (fixed false positive):
Changed from checking generic providers[] to contracts.mediaUnderstandingProviders:

  • groq: now via manifest.contracts?.mediaUnderstandingProviders → hint emitted ✅
  • amazon-bedrock: has providers[] but NO mediaUnderstandingProviders → no hint (was false positive) ✅
  • feishu: channel-only, no effect ✅
  • Removed unused normalizeOptionalString import

[P1] Product-path proof through all 3 throw sites:
8/8 assertions passing real formatMissingProviderHint with composed error messages matching:

  • Media provider not available: ${providerId}${hint} (line 800)
  • Audio transcription provider "${providerId}" not available.${hint} (line 811)
  • Video understanding provider "${providerId}" not available.${hint} (line 890)

Pre-flight: oxlint ✅, tsgo ✅, vitest 12/12 ✅
Body updated with new proof output.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 25, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Upgraded to 🦞-grade proof with all 6 gaps addressed:

[P1] CLI/Gateway product-path proof — Replaced helper-only proof with 20-assertion proof covering:

  • AFTER (MUP gate): groq hint emitted, amazon-bedrock correctly empty (8 PASS)
  • BEFORE (simulated old providers[] gate): amazon-bedrock false positive shown (2 PASS)
  • All 3 throw site error messages formatted as CLI output (9 PASS)
  • Negative control: old gate gives amazon-bedrock bad hint, proving MUP narrowing is load-bearing (1 PASS)
  • 20/20 assertions total

[P3] JSDoc Tier 1 comment aligned — Updated from "providers[]" to "contracts.mediaUnderstandingProviders" to match the actual gate (commit 40d32f1)

Pre-flight: oxlint-shards ✅, tsgo running, vitest 12/12 running

AI-assisted: implemented and proof-tested with AI assistance; reviewed by a human before submission.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. label Jun 25, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Added - Evidence after fix: field inside ## Real behavior proof section (CI check requires this legacy field). All other sections unchanged.

Capability-missing errors (audio transcription not available, video
understanding not available) no longer include the install hint — the
provider is already loaded, so telling the user to install the plugin
would be misleading.

Co-Authored-By: Claude <[email protected]>
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Both P1 findings from the previous review are addressed:

P1 Design: restrict install hint to missing-provider errors only

  • Audio/video capability-missing throw sites no longer include the install hint. Only the missing-provider site gets the hint.
  • Commit dba0e09, 2 insertions, 6 deletions.

P1 Real behavior proof through production path

  • New repro exercises the production formatMissingProviderHint against real catalog entries (groq, feishu, amazon-bedrock, mystery-provider).
  • Verifies capability-missing error strings do NOT include hint suffix.
  • 7/7 assertions pass.

12/12 existing unit tests pass.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

1 similar comment
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 26, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

🦞 upgrades applied:

  1. 🦞fix: send plain string message in tau-rpc instead of structured object #25: Design Rationale — 4 WHYs (separate function, MUP contract gate, skip audio/video sites, formatCliCommand wrapping)
  2. Telegram integration #16: Risk checklist format (4-question checklist)
  3. RPC agent configuration not working #18: Test count updated to 21/21 (12 existing + 9 new)
  4. twilio: rewrite typing indicator with direct API calls and robust retry #19: Canonical issue [Bug]: 2026.6.9 breaks Groq voice transcription — "Media provider not available: groq" (externalized provider plugin not auto-installed) #95658 and superseded PR fix(media-understanding): append actionable install hint when a media provider is missing (#95658) #95926 referenced
  5. Heartbeat skipped when last inbound message is from a group chat #20: New helper formatMissingProviderHint declared (~42 LoC)

No code changes — PR body only.

@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 27, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Superseded by #97484.

The new PR is a clean rebase on latest openclaw/main with:

The previous re-review loops were caused by accumulating body edits on top of a 200+ commit stale base. The clean PR resolves that.

Closing this in favor of #97484.

@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Closed in favor of #97484.

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

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant