From d492bb93684aac62c52fb2c1ece4130bf82196ea Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 18 Sep 2026 13:41:34 -0700 Subject: [PATCH] fix(media): report CLI models with missing command or args (#151969) CLI media models with a missing command or empty argument list passed Doctor. When a command existed but args were absent, the runtime invoked the transcriber without the attachment. Doctor and execution now share the media resolver's minimum-input check. Doctor reports the exact configuration path and a manual fix; execution records a typed refusal before spawning and continues configured fallbacks. Gateway startup validation remains unchanged. Custom and literal arguments still work, and a valid command returning empty stdout still completes. The Discord fixture now supplies attachment arguments for its empty-output case and waits for owned work to settle on refusals, preserving warning, no-dispatch, and WAV-cleanup coverage. Release-note context: report incomplete CLI media models in Doctor and prevent command-only transcription attempts while preserving Gateway startup, custom arguments, and configured fallbacks. Fixes #151883. Thanks to @fede-kamel for reporting and clarifying the missing-args failure. No reporter code was adopted. Validation: 203 focused tests reported passing, check:changed passing, clean Codex review, and exact-head CI run 35385552484 successful. Landing REST verification found 217 completed checks: 170 successful, 46 skipped, one neutral, and no failures or pending checks. The follow-up from the reviewed production head changes only the Discord integration test. The published-updater compatibility check was explicitly skipped by maintainer decision: Doctor adds advisory diagnostics only, with no persisted-shape, migration, update-path, or service-lifecycle change. No updater compatibility run is claimed. --- docs/nodes/media-understanding.md | 2 + src/commands/doctor-config-analysis.ts | 24 +++ src/commands/doctor-config-flow.test.ts | 79 +++++---- src/commands/doctor-config-flow.ts | 2 + src/media-understanding/resolve.ts | 76 ++++++++- .../runner.cli-audio.test.ts | 150 ++++++++++++++++++ src/media-understanding/runner.entries.ts | 49 +----- .../runtime.audio-disposition.test.ts | 7 +- ...nscripts-discord-audio.integration.test.ts | 71 +++++++-- 9 files changed, 362 insertions(+), 98 deletions(-) diff --git a/docs/nodes/media-understanding.md b/docs/nodes/media-understanding.md index a030b7df3554..4ac1eb9fcd12 100644 --- a/docs/nodes/media-understanding.md +++ b/docs/nodes/media-understanding.md @@ -115,6 +115,8 @@ Each `models[]` entry is a **provider** entry (default) or a **CLI** entry: +CLI entries need a nonblank `command` and a nonempty `args` list. Arguments remain literal strings with optional template interpolation; existing literal file paths and custom wrapper arguments are supported. Pass the attachment through a template such as `{{AttachmentPath}}` or your command's existing input contract. Empty argument lists are not supported because OpenClaw does not feed attachments to CLI stdin. `openclaw doctor` reports missing commands or args with the exact config path and a manual fix; it does not invent commands or rewrite these entries. At runtime, an incomplete entry records a failure without launching the binary, and the next configured model is tried. If none succeeds, the attachment gets a failure outcome and a warning is logged. Config validation remains permissive for these fields so an existing config can still start the Gateway after an update. + ### Provider credentials Provider media understanding uses the same auth resolution as normal model calls: auth profiles, environment variables, then `models.providers..apiKey`. `tools.media.models[]` entries do not accept an inline `apiKey` field. diff --git a/src/commands/doctor-config-analysis.ts b/src/commands/doctor-config-analysis.ts index d48045cf33f2..a538346ac1cf 100644 --- a/src/commands/doctor-config-analysis.ts +++ b/src/commands/doctor-config-analysis.ts @@ -18,11 +18,35 @@ import type { ConfigFileSnapshot, OpenClawConfig } from "../config/types.opencla import { OpenClawSchema } from "../config/zod-schema.js"; import { isPathInside } from "../infra/path-guards.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; +import { resolveCliModelEntry } from "../media-understanding/resolve.js"; import { isRecord } from "../utils.js"; import { sanitizeDoctorNote } from "./doctor/emit-notes.js"; const configLog = createSubsystemLogger("config"); +export function noteMediaCliModelWarnings(cfg: OpenClawConfig): void { + const models = cfg.tools?.media?.models; + if (!Array.isArray(models)) { + return; + } + const warnings: string[] = []; + models.forEach((entry, index) => { + if (!entry || (entry.type ?? (entry.command ? "cli" : "provider")) !== "cli") { + return; + } + const resolved = resolveCliModelEntry(entry); + if (!resolved.ok) { + const field = resolved.error.reason === "cli-missing-command" ? "command" : "args"; + warnings.push( + `- tools.media.models[${index}].${field}: Invalid CLI media model. ${resolved.error.message} Doctor cannot choose a command or attachment arguments; edit this entry.`, + ); + } + }); + if (warnings.length > 0) { + note(warnings.join("\n"), "Doctor warnings"); + } +} + export function noteDoctorConfigPreflightIssues( snapshot: ConfigFileSnapshot, options: { invalidConfigNote?: string | false; activeRepair: boolean }, diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index e71d9f3bfea1..c17dcc59bd1c 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -6,6 +6,7 @@ import { withTempHome } from "openclaw/plugin-sdk/test-env"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { migratePersistedImplicitMainRoster } from "../config/legacy.roster.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { MediaUnderstandingModelConfig } from "../config/types.tools.js"; import { writeChannelPairingStateSnapshot } from "../pairing/pairing-store-sqlite.test-helpers.js"; import type { PluginCapabilityConsentHandler } from "../plugins/capability-consent.js"; import { buildPluginCapabilityConsentReview } from "../plugins/capability-summary.js"; @@ -1389,51 +1390,18 @@ vi.mock("./doctor-config-preflight.js", async () => { }); vi.mock("./doctor-config-analysis.js", async (importOriginal) => { - const { noteDoctorHookConfigWarnings, noteMissingDefaultAgentOwner } = - await importOriginal(); - function formatConfigKeyPath(parts: Array): string { - if (parts.length === 0) { - return ""; - } - let out = ""; - for (const part of parts) { - if (typeof part === "number") { - out += `[${part}]`; - } else { - out = out ? `${out}.${part}` : part; - } - } - return out || ""; - } - - function resolveConfigPathTarget(root: unknown, pathParts: Array): unknown { - let current: unknown = root; - for (const part of pathParts) { - if (typeof part === "number") { - if (!Array.isArray(current)) { - return null; - } - current = current[part]; - continue; - } - if (!current || typeof current !== "object" || Array.isArray(current)) { - return null; - } - current = (current as Record)[part]; - } - return current; - } + const actual = await importOriginal(); return { - collectImplicitFallbackClobberWarnings: collectImplicitFallbackClobberWarningsMock, - formatConfigKeyPath, + formatConfigKeyPath: actual.formatConfigKeyPath, noteImplicitFallbackClobberWarnings: noteImplicitFallbackClobberWarningsMock, noteOpencodeProviderOverrides: vi.fn(), noteMcpOriginWarning: vi.fn(), - noteDoctorHookConfigWarnings, - noteMissingDefaultAgentOwner, + noteDoctorHookConfigWarnings: actual.noteDoctorHookConfigWarnings, + noteMediaCliModelWarnings: actual.noteMediaCliModelWarnings, + noteMissingDefaultAgentOwner: actual.noteMissingDefaultAgentOwner, noteSandboxOriginProxyWarning: vi.fn(), - resolveConfigPathTarget, + resolveConfigPathTarget: actual.resolveConfigPathTarget, stripUnknownConfigKeys: vi.fn((config: Record) => { const next = structuredClone(config); const removed: string[] = []; @@ -1441,7 +1409,7 @@ vi.mock("./doctor-config-analysis.js", async (importOriginal) => { delete next.bridge; removed.push("bridge"); } - const gatewayAuth = resolveConfigPathTarget(next, ["gateway", "auth"]); + const gatewayAuth = actual.resolveConfigPathTarget(next, ["gateway", "auth"]); if ( gatewayAuth && typeof gatewayAuth === "object" && @@ -2424,6 +2392,37 @@ describe("doctor config flow", () => { expect(doctorWarnings.join("\n")).toContain("clobbers agents.defaults.model.fallbacks"); }); + it.each([false, true])( + "reports invalid CLI media models without repairing them (repair=%s)", + async (repair) => { + const models = [ + { provider: "fixture-provider", capabilities: ["audio"] }, + { type: "cli", capabilities: ["audio"] }, + { type: "cli", command: "fixture-transcribe", capabilities: ["audio"] }, + { type: "cli", command: "fixture-transcribe", args: ["{{AttachmentPath}}"] }, + { command: "fixture-transcribe", args: ["/synthetic/audio.wav"] }, + ] satisfies MediaUnderstandingModelConfig[]; + const config: OpenClawConfig = { plugins: { enabled: false }, tools: { media: { models } } }; + config.agents = { entries: { main: {} } }; + const result = await runDoctorConfigWithInput({ + config, + repair, + run: loadAndMaybeMigrateDoctorConfig, + }); + const warnings = terminalNoteMock.mock.calls + .filter(([, title]) => title === "Doctor warnings") + .map(([message]) => message) + .join("\n"); + expect(warnings).toContain("tools.media.models[1].command"); + expect(warnings).toContain("tools.media.models[2].args"); + expect(warnings).toContain("{{AttachmentPath}}"); + expect(warnings).toContain("Doctor cannot choose"); + expect(warnings).not.toMatch(/tools\.media\.models\[(?:0|3|4)\]/); + expect(result.cfg.tools?.media).toEqual(config.tools?.media); + expect(result.shouldWriteConfig, result.pendingChangePanels?.join("\n")).toBe(false); + }, + ); + it("warns when internal hook entries include unsupported loader keys", async () => { const doctorWarnings = await collectDoctorWarnings({ hooks: { diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 38eb61fde020..d62f853faa33 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -27,6 +27,7 @@ import { noteDoctorHookConfigWarnings, noteImplicitFallbackClobberWarnings, noteMcpOriginWarning, + noteMediaCliModelWarnings, noteMissingDefaultAgentOwner, noteOpencodeProviderOverrides, noteSandboxOriginProxyWarning, @@ -679,6 +680,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { noteImplicitFallbackClobberWarnings(cfg); noteSandboxOriginProxyWarning(cfg); noteMcpOriginWarning(cfg); + noteMediaCliModelWarnings(cfg); noteMissingDefaultAgentOwner(cfg); const migrationResult = await finalizeMigrationResult({ diff --git a/src/media-understanding/resolve.ts b/src/media-understanding/resolve.ts index 1581ef060a6b..34a10856147b 100644 --- a/src/media-understanding/resolve.ts +++ b/src/media-understanding/resolve.ts @@ -4,6 +4,8 @@ import { MAX_TIMER_TIMEOUT_MS, resolveTimerTimeoutMs, } from "@openclaw/normalization-core/number-coercion"; +import { err, ok, type Result } from "@openclaw/normalization-core/result"; +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import type { MsgContext } from "../auto-reply/templating.js"; import type { OpenClawConfig } from "../config/types.js"; import type { @@ -18,6 +20,7 @@ import { DEFAULT_MAX_CHARS_BY_CAPABILITY, DEFAULT_MEDIA_CONCURRENCY, DEFAULT_PROMPT, + DEFAULT_TIMEOUT_SECONDS, } from "./defaults.constants.js"; import { resolveEffectiveMediaEntryCapabilities } from "./entry-capabilities.js"; import { normalizeMediaUnderstandingChatType, resolveMediaUnderstandingScope } from "./scope.js"; @@ -28,6 +31,42 @@ export type ResolvedMediaModelEntry = { secretOwnerId?: string; }; +class MediaCliModelUnavailableError extends Error { + constructor( + readonly reason: "cli-missing-command" | "cli-missing-attachment-arg", + message: string, + ) { + super(`${reason}; ${message}`); + } +} + +/** Resolve executable CLI inputs without making invalid media config startup-fatal. */ +export function resolveCliModelEntry( + entry: MediaUnderstandingModelConfig, +): Result<{ command: string; args: string[] }, MediaCliModelUnavailableError> { + const command = normalizeOptionalString(entry.command); + if (!command) { + return err( + new MediaCliModelUnavailableError( + "cli-missing-command", + 'Set command to the media executable and args to pass the attachment, for example ["{{AttachmentPath}}"].', + ), + ); + } + const args = entry.args; + // No stdin is supplied, so empty args cannot carry the attachment. Nonempty + // literal/custom argv is a shipped command contract; interpolation is optional. + if (!Array.isArray(args) || args.length === 0) { + return err( + new MediaCliModelUnavailableError( + "cli-missing-attachment-arg", + 'Set args to pass the attachment, for example ["{{AttachmentPath}}"]. CLI stdin is not supplied.', + ), + ); + } + return ok({ command, args }); +} + /** Default per-provider media-understanding runtime timeout in milliseconds. */ const DEFAULT_MEDIA_RUNTIME_TIMEOUT_MS = 30_000; const MIN_MEDIA_TIMEOUT_MS = 1000; @@ -52,7 +91,7 @@ export function resolveMediaRuntimeTimeoutMs(timeoutMs: number | undefined): num } /** Resolves the provider prompt and appends length guidance for non-audio outputs. */ -export function resolvePrompt( +function resolvePrompt( capability: MediaUnderstandingCapability, prompt?: string, maxChars?: number, @@ -65,7 +104,7 @@ export function resolvePrompt( } /** Resolves the effective max response characters for a model entry and capability. */ -export function resolveMaxChars(params: { +function resolveMaxChars(params: { capability: MediaUnderstandingCapability; entry: MediaUnderstandingModelConfig; cfg: OpenClawConfig; @@ -97,6 +136,39 @@ export function resolveMaxBytes(params: { return DEFAULT_MAX_BYTES[params.capability]; } +export function resolveEntryRunOptions(params: { + capability: MediaUnderstandingCapability; + entry: MediaUnderstandingModelConfig; + cfg: OpenClawConfig; + config?: MediaUnderstandingConfig; +}): { + maxBytes: number; + maxChars?: number; + timeoutMs: number; + prompt: string; + hasConfiguredPrompt: boolean; +} { + const { capability, entry, cfg } = params; + const maxBytes = resolveMaxBytes({ capability, entry, cfg, config: params.config }); + const maxChars = resolveMaxChars({ capability, entry, cfg, config: params.config }); + const timeoutMs = resolveTimeoutMs( + entry.timeoutSeconds ?? + params.config?.timeoutSeconds ?? + cfg.tools?.media?.[capability]?.timeoutSeconds, + DEFAULT_TIMEOUT_SECONDS[capability], + ); + const configuredPrompt = + entry.prompt ?? params.config?.prompt ?? cfg.tools?.media?.[capability]?.prompt; + const prompt = resolvePrompt(capability, configuredPrompt, maxChars); + return { + maxBytes, + maxChars, + timeoutMs, + prompt, + hasConfiguredPrompt: Boolean(configuredPrompt?.trim()), + }; +} + /** Maps the message context to an allow/deny decision for configured media scope rules. */ export function resolveScopeDecision(params: { scope?: MediaUnderstandingScopeConfig; diff --git a/src/media-understanding/runner.cli-audio.test.ts b/src/media-understanding/runner.cli-audio.test.ts index 5c62a74bed07..1e33bd998c1a 100644 --- a/src/media-understanding/runner.cli-audio.test.ts +++ b/src/media-understanding/runner.cli-audio.test.ts @@ -4,6 +4,8 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/types.js"; +import type { MediaUnderstandingModelConfig } from "../config/types.tools.js"; +import { logWarn } from "../logger.js"; import { withTestDir } from "../test-helpers/temp-dir.js"; import { withEnvAsync } from "../test-utils/env.js"; import { CLI_OUTPUT_MAX_BUFFER } from "./defaults.constants.js"; @@ -18,6 +20,11 @@ import type { MediaAttachment } from "./types.js"; const runExecMock = vi.hoisted(() => vi.fn()); const runFfmpegMock = vi.hoisted(() => vi.fn()); +vi.mock("../logger.js", async (importOriginal) => ({ + ...(await importOriginal()), + logWarn: vi.fn(), +})); + vi.mock("../process/exec.js", () => ({ runExec: (...args: unknown[]) => runExecMock(...args), })); @@ -136,6 +143,149 @@ describe("media-understanding CLI audio entry", () => { vi.clearAllMocks(); }); + it.each<{ name: string; entry: MediaUnderstandingModelConfig; reason: string }>([ + { name: "missing command", entry: { type: "cli" }, reason: "cli-missing-command" }, + { name: "blank command", entry: { type: "cli", command: " " }, reason: "cli-missing-command" }, + { + name: "missing args", + entry: { type: "cli", command: "fixture-transcribe" }, + reason: "cli-missing-attachment-arg", + }, + { + name: "empty args", + entry: { command: "fixture-transcribe", args: [] }, + reason: "cli-missing-attachment-arg", + }, + ])("reports $name as unavailable without executing it", async ({ entry, reason }) => { + const { runCapability } = await import("./runner.js"); + await withAudioFixture("openclaw-cli-unavailable", async ({ ctx, media, cache }) => { + await expect( + runCliEntry({ + capability: "audio", + entry, + cfg: {}, + ctx, + attachment: requireFirstAttachment(media), + cache, + }), + ).rejects.toMatchObject({ reason }); + const result = await runCapability({ + capability: "audio", + cfg: { tools: { media: { models: [{ ...entry, capabilities: ["audio"] }] } } }, + ctx, + media, + attachments: cache, + providerRegistry: new Map(), + }); + expect(result.outputs).toEqual([]); + expect(result.decision).toMatchObject({ + outcome: "failed", + attachmentProcessing: { 0: "omitted" }, + attachmentDispositions: { 0: { kind: "failed" } }, + attachments: [ + { + attempts: [{ type: "cli", outcome: "failed", reason: expect.stringContaining(reason) }], + }, + ], + }); + expect(logWarn).toHaveBeenCalledWith(expect.stringContaining(reason)); + expect(runExecMock).not.toHaveBeenCalled(); + expect(runFfmpegMock).not.toHaveBeenCalled(); + }); + }); + + it("continues to a working CLI after an unavailable entry", async () => { + const { runCapability } = await import("./runner.js"); + await withAudioFixture("openclaw-cli-fallback", async ({ ctx, media, cache }) => { + const result = await runCapability({ + capability: "audio", + cfg: { + tools: { + media: { + models: [ + { type: "cli", command: "invalid-transcribe", capabilities: ["audio"] }, + { + type: "cli", + command: "working-transcribe", + args: ["{{ AttachmentPath }}"], + capabilities: ["audio"], + }, + ], + }, + }, + }, + ctx, + media, + attachments: cache, + providerRegistry: new Map(), + }); + expect(result.outputs[0]?.text).toBe("cli transcript"); + expect(result.decision.attachments[0]?.attempts.map((attempt) => attempt.outcome)).toEqual([ + "failed", + "success", + ]); + expect(runExecMock).toHaveBeenCalledExactlyOnceWith( + "working-transcribe", + [expect.any(String)], + expect.any(Object), + ); + }); + }); + + it("executes a custom CLI with a literal attachment path", async () => { + const actual = await vi.importActual("../process/exec.js"); + runExecMock.mockImplementationOnce(actual.runExec); + await withAudioFixture( + "openclaw-cli-literal-input", + async ({ ctx, media, mediaPath, cache }) => { + const args = [ + "-e", + "process.stdout.write(String(require('node:fs').readFileSync(process.argv[1]).length))", + mediaPath, + ]; + const result = await runCliEntry({ + capability: "audio", + entry: { type: "cli", command: process.execPath, args }, + cfg: {}, + ctx, + attachment: requireFirstAttachment(media), + cache, + }); + expect(result?.text).toBe(String((await fs.stat(mediaPath)).size)); + expect(runExecMock).toHaveBeenCalledExactlyOnceWith( + process.execPath, + args, + expect.any(Object), + ); + }, + ); + }); + + it("preserves custom arguments relative to the attachment working directory", async () => { + await withAudioFixture( + "openclaw-cli-custom-input", + async ({ ctx, media, mediaPath, cache }) => { + const args = ["describe", path.basename(mediaPath)]; + const result = await runCliEntry({ + capability: "audio", + entry: { type: "cli", command: "agy", args }, + cfg: {}, + ctx, + attachment: requireFirstAttachment(media), + cache, + }); + expect(result?.text).toBe("cli transcript"); + expect(runExecMock).toHaveBeenCalledExactlyOnceWith( + "agy", + args, + expect.objectContaining({ + cwd: path.dirname(await fs.realpath(mediaPath)), + }), + ); + }, + ); + }); + it("applies per-request prompt and language overrides to CLI transcription templating", async () => { let mediaPath = ""; diff --git a/src/media-understanding/runner.entries.ts b/src/media-understanding/runner.entries.ts index 6f84b42a4bf0..435086623c2e 100644 --- a/src/media-understanding/runner.entries.ts +++ b/src/media-understanding/runner.entries.ts @@ -54,11 +54,7 @@ import { assertSecretOwnerAvailable } from "../secrets/runtime-degraded-state.js import { assertRuntimeMediaRequestSecretOwnerAvailable } from "../secrets/runtime-media-secret-owner.js"; import { createLazyRuntimeModule } from "../shared/lazy-runtime.js"; import { MediaAttachmentCache } from "./attachments.js"; -import { - CLI_OUTPUT_MAX_BUFFER, - DEFAULT_TIMEOUT_SECONDS, - MIN_AUDIO_FILE_BYTES, -} from "./defaults.constants.js"; +import { CLI_OUTPUT_MAX_BUFFER, MIN_AUDIO_FILE_BYTES } from "./defaults.constants.js"; import { normalizeImageDescriptionInput, optimizeImageDescriptionInput, @@ -70,7 +66,7 @@ import { } from "./local-audio.js"; import { resolveOpenAiAudioAuthModelApi } from "./openai-audio-api.js"; import { getMediaUnderstandingProvider, normalizeMediaProviderId } from "./provider-registry.js"; -import { resolveMaxBytes, resolveMaxChars, resolvePrompt, resolveTimeoutMs } from "./resolve.js"; +import { resolveCliModelEntry, resolveEntryRunOptions } from "./resolve.js"; import type { AudioTranscriptionResult, MediaAttachment, @@ -407,39 +403,6 @@ export function buildModelDecision(params: { }; } -function resolveEntryRunOptions(params: { - capability: MediaUnderstandingCapability; - entry: MediaUnderstandingModelConfig; - cfg: OpenClawConfig; - config?: MediaUnderstandingConfig; -}): { - maxBytes: number; - maxChars?: number; - timeoutMs: number; - prompt: string; - hasConfiguredPrompt: boolean; -} { - const { capability, entry, cfg } = params; - const maxBytes = resolveMaxBytes({ capability, entry, cfg, config: params.config }); - const maxChars = resolveMaxChars({ capability, entry, cfg, config: params.config }); - const timeoutMs = resolveTimeoutMs( - entry.timeoutSeconds ?? - params.config?.timeoutSeconds ?? - cfg.tools?.media?.[capability]?.timeoutSeconds, - DEFAULT_TIMEOUT_SECONDS[capability], - ); - const configuredPrompt = - entry.prompt ?? params.config?.prompt ?? cfg.tools?.media?.[capability]?.prompt; - const prompt = resolvePrompt(capability, configuredPrompt, maxChars); - return { - maxBytes, - maxChars, - timeoutMs, - prompt, - hasConfiguredPrompt: Boolean(configuredPrompt?.trim()), - }; -} - function resolveMediaRequestOverrides(config: MediaUnderstandingConfig | undefined): { prompt?: string; language?: string; @@ -1051,11 +1014,11 @@ export async function runCliEntry(params: { }): Promise { const { entry, capability, cfg, ctx } = params; const attachmentIndex = params.attachment.index; - const command = entry.command?.trim(); - const args = entry.args ?? []; - if (!command) { - throw new Error(`CLI entry missing command for ${capability}`); + const cli = resolveCliModelEntry(entry); + if (!cli.ok) { + throw cli.error; } + const { command, args } = cli.value; const requestOverrides = resolveMediaRequestOverrides(params.config); const language = requestOverrides.language ?? entry.language ?? params.config?.language; const { maxBytes, maxChars, timeoutMs, prompt } = resolveEntryRunOptions({ diff --git a/src/media-understanding/runtime.audio-disposition.test.ts b/src/media-understanding/runtime.audio-disposition.test.ts index c6f04e1ede9a..44d9ddb1c130 100644 --- a/src/media-understanding/runtime.audio-disposition.test.ts +++ b/src/media-understanding/runtime.audio-disposition.test.ts @@ -42,7 +42,12 @@ describe("audio processing disposition", () => { }); const models: MediaUnderstandingModelConfig[] = entries.map((entry) => entry === "cli" - ? { type: "cli", command: "synthetic-stt", capabilities: ["audio"] } + ? { + type: "cli", + command: "synthetic-stt", + args: ["{{AttachmentPath}}"], + capabilities: ["audio"], + } : { provider: "synthetic-audio", model: "synthetic-stt", diff --git a/test/transcripts-discord-audio.integration.test.ts b/test/transcripts-discord-audio.integration.test.ts index 44b77a636290..be05abc3ae01 100644 --- a/test/transcripts-discord-audio.integration.test.ts +++ b/test/transcripts-discord-audio.integration.test.ts @@ -28,6 +28,8 @@ defineDiscordVoiceTests( transcribeAudioFileMock, agentCommandMock, controlRealtimeVoiceAgentRunMock, + loggerWarnMock, + receiveRecordedSpeech, }) => { it.each( ["conversation", "control"].flatMap((dispatch) => @@ -73,7 +75,15 @@ defineDiscordVoiceTests( }; const models: MediaUnderstandingModelConfig[] = opening === "empty CLI" - ? [{ type: "cli", command: "synthetic-stt", capabilities: ["audio"] }] + ? [ + { + type: "cli", + command: "synthetic-stt", + // The process succeeds with empty stdout; its input is still valid. + args: ["{{AttachmentPath}}"], + capabilities: ["audio"], + }, + ] : [ ...(opening === "fallback" ? [{ ...apiModel, model: "unavailable-stt" }] : []), apiModel, @@ -102,14 +112,9 @@ defineDiscordVoiceTests( ); await manager.join({ guildId: "g1", channelId: "1001" }); const entry = getSessionEntry(manager); - const dispatched = createDeferred(); - agentCommandMock.mockImplementation(async () => { - dispatched.resolve(); - return { payloads: [] }; - }); + const conversations = vi.spyOn(entry.conversations, "enqueue"); if (dispatch === "control") { controlRealtimeVoiceAgentRunMock.mockImplementation(async () => { - dispatched.resolve(); return { ok: true, mode: "cancel", @@ -181,8 +186,14 @@ defineDiscordVoiceTests( } recordingStream.end(); await receivingOutcome; - await recorded.promise; + if (oversized) { + // The capture scan owns a new receive operation after the rejected opening. + await recorded.promise; + } await entry.processingQueue; + // Join owned work even when transcription fails and produces no sink/dispatch call. + await Promise.all(conversations.mock.results.map((result) => result.value)); + expect(loggerWarnMock).not.toHaveBeenCalledWith(expect.stringContaining("cli-missing-")); expect(wavSizes).toEqual(oversized ? [192_044] : [192_044, 192_044]); expect(results[0]).toMatchObject({ text: oversized ? suffix : opening === "fallback" ? prefix : undefined, @@ -205,10 +216,6 @@ defineDiscordVoiceTests( await expect(fs.stat(wavPath)).rejects.toMatchObject({ code: "ENOENT" }); } const completeText = opening === "fallback" ? `${prefix}\n${suffix}` : suffix; - if (!oversized) { - // Recording completion does not join the independent conversation queue. - await dispatched.promise; - } if (oversized) { expect(agentCommandMock).not.toHaveBeenCalled(); expect(controlRealtimeVoiceAgentRunMock).not.toHaveBeenCalled(); @@ -231,9 +238,49 @@ defineDiscordVoiceTests( recordingStream.end(); await receivingOutcome; await entry.processingQueue; + conversations.mockRestore(); await manager.destroy(); } }, ); + + it.each([ + { command: undefined, args: ["{{AttachmentPath}}"], reason: "cli-missing-command" }, + { command: "synthetic-stt", args: undefined, reason: "cli-missing-attachment-arg" }, + ])("finishes voice capture with a warning for $reason", async ({ command, args, reason }) => { + cli.mockReset(); + const manager = createManager( + makeVoiceConfig({}, { groupPolicy: "open", allowFrom: ["discord:u-owner"] }), + undefined, + { tools: { media: { models: [{ type: "cli", command, args, capabilities: ["audio"] }] } } }, + ); + await manager.join({ guildId: "g1", channelId: "1001" }); + const sink = vi.fn(); + expect(await startTranscripts(manager, sink)).toMatchObject({ ok: true }); + const wavPaths: string[] = []; + transcribeAudioFileMock.mockImplementation(async (params) => { + wavPaths.push(params.filePath); + return await withPluginRuntimeGenerationScope( + { + metadataSnapshot: createPluginMetadataSnapshotFixture({ plugins: [] }), + pluginRegistry: createEmptyPluginRegistry(), + }, + () => transcribeAudioFile(params), + ); + }); + // This joins receive, recording, and conversation completion, including refusal. + await receiveRecordedSpeech(manager); + expect(transcribeAudioFileMock).toHaveBeenCalledOnce(); + expect(cli).not.toHaveBeenCalled(); + expect(loggerWarnMock).toHaveBeenCalledWith( + expect.stringContaining(`discord voice: recording failed: ${reason}; Set `), + ); + expect(sink).not.toHaveBeenCalled(); + expect(agentCommandMock).not.toHaveBeenCalled(); + expect(controlRealtimeVoiceAgentRunMock).not.toHaveBeenCalled(); + for (const wavPath of wavPaths) { + await expect(fs.stat(wavPath)).rejects.toMatchObject({ code: "ENOENT" }); + } + }); }, );