Skip to content

Commit c562ca7

Browse files
authored
Merge pull request #7050 from nexu-io/backport-6993-to-release/v0.20.0
[backport release/v0.20.0] fix(daemon): carry Vela image safety refusals through to the client
2 parents 0e88f94 + 4795059 commit c562ca7

13 files changed

Lines changed: 1024 additions & 50 deletions

File tree

apps/daemon/src/integrations/vela-command.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,41 @@ import {
1818
} from '../runtimes/launch.js';
1919
import { getAgentDef } from '../runtimes/registry.js';
2020

21+
/**
22+
* A failed Vela command still carries meaning on stdout.
23+
*
24+
* `vela image gen --json` prints the complete task object — including its
25+
* structured `error` block — and only then exits non-zero. Rejecting with the
26+
* bare exec error therefore threw away the one thing that distinguishes a
27+
* content-safety rejection from a provider outage, leaving every caller with
28+
* nothing but "Command failed with exit code 1".
29+
*
30+
* The stdout is attached to the rejection rather than resolved, so the command
31+
* still fails and no caller can mistake a failure for success; readers that
32+
* want the structured detail opt in through `velaCommandStdout`.
33+
*/
34+
function withCommandStdout(error: unknown, stdout: string): unknown {
35+
if (!stdout || typeof error !== 'object' || error === null) return error;
36+
try {
37+
(error as { stdout?: string }).stdout = stdout;
38+
} catch {
39+
// A frozen or exotic error object must not turn a command failure into a
40+
// different, more confusing failure.
41+
}
42+
return error;
43+
}
44+
45+
/**
46+
* Read the stdout captured by `withCommandStdout` off a rejected Vela command.
47+
* Returns an empty string when the failure carried none (a crash before any
48+
* output, or a non-object rejection).
49+
*/
50+
export function velaCommandStdout(error: unknown): string {
51+
if (typeof error !== 'object' || error === null) return '';
52+
const stdout = (error as { stdout?: unknown }).stdout;
53+
return typeof stdout === 'string' ? stdout : '';
54+
}
55+
2156
export interface VelaCommandOptions {
2257
env?: NodeJS.ProcessEnv;
2358
configuredEnv?: Record<string, string>;
@@ -276,7 +311,7 @@ export function runVelaCommand(
276311
// Diagnostics are observational and must never change transport.
277312
}
278313
}
279-
if (error) settle({ error });
314+
if (error) settle({ error: withCommandStdout(error, stdout) });
280315
else settle({ stdout });
281316
},
282317
);

apps/daemon/src/media/tasks.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,26 @@ export type MediaTaskStatus =
77
| 'failed'
88
| 'interrupted';
99

10+
/** What a content-safety policy objected to, when the supplier proved it. */
11+
export type MediaTaskErrorSubject = 'prompt' | 'input_image' | 'output_image';
12+
1013
export interface MediaTaskError {
1114
message: string;
1215
status?: number;
1316
code?: string;
17+
/**
18+
* Optional hint about what a content-safety policy objected to, forwarded
19+
* only when the upstream supplier proved it. Absent means "not proven", not
20+
* "not applicable" — a client must then name both the prompt and the
21+
* reference images rather than blame one of them.
22+
*/
23+
subject?: MediaTaskErrorSubject;
24+
/**
25+
* Whether repeating the identical request could plausibly behave
26+
* differently. Undefined means the producer did not say; only an explicit
27+
* `false` licenses telling a user that retrying is pointless.
28+
*/
29+
retryable?: boolean;
1430
}
1531

1632
export interface MediaTaskRow {
@@ -309,6 +325,29 @@ function parseArray(json: string | null): string[] {
309325
: [];
310326
}
311327

328+
/**
329+
* Subjects a content-safety refusal may name. Validated on read as well as on
330+
* write: the value crosses a JSON column, so a row written by a newer daemon
331+
* (or hand-edited) must not smuggle an unknown subject back into the API
332+
* response.
333+
*/
334+
const MEDIA_TASK_ERROR_SUBJECTS = ['prompt', 'input_image', 'output_image'] as const;
335+
336+
function isMediaTaskErrorSubject(value: unknown): value is MediaTaskErrorSubject {
337+
return (
338+
typeof value === 'string'
339+
&& (MEDIA_TASK_ERROR_SUBJECTS as readonly string[]).includes(value)
340+
);
341+
}
342+
343+
/**
344+
* Rebuild a persisted error. Every field the write path stores has to be
345+
* reconstructed here or it silently disappears the moment a task is read back
346+
* from SQLite -- which is every daemon restart and every cache rehydration,
347+
* not an edge case. `subject` and `retryable` were lost that way: a refusal
348+
* survived until the process bounced, then reappeared as a bare failure with
349+
* no attribution and no retry verdict.
350+
*/
312351
function normalizeError(value: unknown): MediaTaskError | null {
313352
if (!value || typeof value !== 'object' || Array.isArray(value)) return null;
314353
const obj = value as Record<string, unknown>;
@@ -317,6 +356,11 @@ function normalizeError(value: unknown): MediaTaskError | null {
317356
const error: MediaTaskError = { message };
318357
if (typeof obj.status === 'number') error.status = obj.status;
319358
if (typeof obj.code === 'string') error.code = obj.code;
359+
if (isMediaTaskErrorSubject(obj.subject)) error.subject = obj.subject;
360+
// Only an explicit boolean survives: absent must stay absent, because
361+
// "the producer did not say" and "the producer said retrying is pointless"
362+
// are different answers to a client.
363+
if (typeof obj.retryable === 'boolean') error.retryable = obj.retryable;
320364
return error;
321365
}
322366

apps/daemon/src/media/vela.ts

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from 'node:path';
44

55
import {
66
runVelaCommand,
7+
velaCommandStdout,
78
velaWorkspaceCommandOptions,
89
} from '../integrations/vela-command.js';
910

@@ -378,10 +379,17 @@ export async function renderVelaImage(
378379
outputPath,
379380
'--json',
380381
];
381-
const stdout = await runCommand(args, {
382-
...velaWorkspaceCommandOptions(input.workspaceId),
383-
timeoutMs: VELA_IMAGE_TIMEOUT_MS,
384-
});
382+
let stdout: string;
383+
try {
384+
stdout = await runCommand(args, {
385+
...velaWorkspaceCommandOptions(input.workspaceId),
386+
timeoutMs: VELA_IMAGE_TIMEOUT_MS,
387+
});
388+
} catch (error) {
389+
// A refused request is a verdict the user can act on, so it must reach
390+
// them as one. Everything else keeps its original error untouched.
391+
throw velaMediaErrorFromFailure(error, `image ${command}`) ?? error;
392+
}
385393
const asset = parseJsonObject(stdout, `image ${command}`);
386394
const assetId = nonEmptyString(asset.asset_id);
387395
const status = nonEmptyString(asset.status);
@@ -412,6 +420,97 @@ export async function renderVelaImage(
412420
}
413421
}
414422

423+
/**
424+
* The stable, provider-neutral code Vela publishes when a content-safety
425+
* policy refused an image request. It is the same string at every layer from
426+
* the provider adapter through the API and the CLI, which is what makes it
427+
* safe to key product behaviour on.
428+
*/
429+
export const VELA_SAFETY_REJECTION_CODE = 'safety_rejection';
430+
431+
/**
432+
* Optional, non-authoritative hint about what a safety policy objected to.
433+
* Absent whenever the upstream supplier could not prove it — callers must then
434+
* fall back to naming both possibilities rather than picking one.
435+
*/
436+
export type VelaSafetySubject = 'prompt' | 'input_image' | 'output_image';
437+
438+
const VELA_SAFETY_SUBJECTS: readonly string[] = [
439+
'prompt',
440+
'input_image',
441+
'output_image',
442+
];
443+
444+
/**
445+
* A Vela media failure that arrived with a machine-readable verdict rather
446+
* than only a human sentence.
447+
*
448+
* `code` is carried on the error itself because the media task route copies
449+
* `err.code` straight into the persisted task snapshot; that is what lets the
450+
* web client render a definite explanation instead of depending on the agent
451+
* to repeat one correctly.
452+
*/
453+
export class VelaMediaError extends Error {
454+
readonly code: string;
455+
readonly subject: VelaSafetySubject | undefined;
456+
readonly retryable: boolean | undefined;
457+
458+
constructor(
459+
message: string,
460+
detail: {
461+
code: string;
462+
subject?: VelaSafetySubject | undefined;
463+
retryable?: boolean | undefined;
464+
},
465+
) {
466+
super(message);
467+
this.name = 'VelaMediaError';
468+
this.code = detail.code;
469+
this.subject = detail.subject;
470+
this.retryable = detail.retryable;
471+
}
472+
}
473+
474+
function safetySubject(value: unknown): VelaSafetySubject | undefined {
475+
return typeof value === 'string' && VELA_SAFETY_SUBJECTS.includes(value)
476+
? (value as VelaSafetySubject)
477+
: undefined;
478+
}
479+
480+
/**
481+
* Rebuild a structured failure from a rejected `vela image --json` run.
482+
*
483+
* Returns undefined for every failure that carried no task JSON — a CLI
484+
* validation error, a crash, a timeout — so those keep their existing generic
485+
* handling. An unrecognised or absent `code` is deliberately NOT promoted to a
486+
* safety rejection: mislabelling an outage as a policy refusal would send a
487+
* user off to rewrite a prompt that was never the problem.
488+
*/
489+
export function velaMediaErrorFromFailure(
490+
error: unknown,
491+
label: string,
492+
): VelaMediaError | undefined {
493+
const stdout = velaCommandStdout(error).trim();
494+
if (!stdout) return undefined;
495+
let parsed: unknown;
496+
try {
497+
parsed = JSON.parse(stdout);
498+
} catch {
499+
return undefined;
500+
}
501+
if (!isRecord(parsed) || !isRecord(parsed.error)) return undefined;
502+
const code = nonEmptyString(parsed.error.code);
503+
if (!code) return undefined;
504+
const message = nonEmptyString(parsed.error.message);
505+
const retryable =
506+
typeof parsed.error.retryable === 'boolean' ? parsed.error.retryable : undefined;
507+
return new VelaMediaError(message ?? `Vela ${label} failed with ${code}`, {
508+
code,
509+
subject: safetySubject(parsed.error.subject),
510+
retryable,
511+
});
512+
}
513+
415514
export async function renderVelaVideo(
416515
input: VelaVideoRenderInput,
417516
runCommand: VelaCommandRunner = runVelaCommand,

apps/daemon/src/prompts/media-contract.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,24 @@ localized sentence and nothing else:
5050
5151
- Success: say the localized equivalent of "Image generated". For Simplified
5252
Chinese, reply exactly \`图片已生成\`.
53-
- Failure, including a placeholder/stub outcome: say the localized equivalent
54-
of "The image generation service is temporarily unavailable". For Simplified
55-
Chinese, reply exactly \`图片生成服务暂时不可用\`.
56-
57-
Do not add a filename, model, provider, reason, remediation, retry offer, or
58-
follow-up question. Use the command's structured result only to choose success
59-
versus failure; retain its original diagnostics in the tool trace for debugging.`;
53+
- Refused by a content safety policy — the structured result's error \`code\` is
54+
\`safety_rejection\`: say the localized equivalent of "The image was not
55+
generated because a content safety policy refused the request". For
56+
Simplified Chinese, reply exactly \`图片未生成:内容安全策略拒绝了该请求\`.
57+
- A structured provider error — the result contains a non-empty error \`code\`
58+
and \`message\`: include both safe fields so the user can understand the
59+
actual failure. For Simplified Chinese, reply exactly
60+
\`图片未生成:{message}(错误代码:{code})\`, substituting the returned values.
61+
- Any other failure, including a placeholder/stub outcome: say the localized
62+
equivalent of "The image generation service is temporarily unavailable". For
63+
Simplified Chinese, reply exactly \`图片生成服务暂时不可用\`.
64+
65+
A provider verdict is not automatically an outage. Use its structured code
66+
and message without reclassifying either one from wording or HTTP status.
67+
68+
Do not add a filename, model, provider, remediation, retry offer, or follow-up
69+
question. For a structured provider error, expose only its safe \`message\` and
70+
\`code\`; retain all other diagnostics in the tool trace for debugging.`;
6071

6172
export function renderMediaGenerationContract(
6273
mediaExecution?: MediaExecutionPolicy | undefined,

apps/daemon/src/routes/media.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { AnalyticsContext } from '../analytics.js';
88
import { defaultMediaExecutionPolicy, mediaPolicyDenial } from '../media/policy.js';
99
import { formatMediaTaskDiagnostic } from '../media/diagnostics.js';
1010
import { findMediaModel } from '../media/models.js';
11+
import type { MediaTaskError } from '../media/tasks.js';
1112
import type { ImageGenerationRequestSummary } from '../media/image-generation-retry.js';
1213
import type { RouteDeps } from '../server-context.js';
1314
import type {
@@ -96,6 +97,32 @@ export function resolveLegacyMediaRouteGrant(input: {
9697
return { ok: true, grant: input.grant };
9798
}
9899

100+
101+
/**
102+
* Build the persisted failure record for a media task.
103+
*
104+
* A media failure is the only thing the client has left to explain itself
105+
* with, so anything the producer proved must survive into the snapshot: the
106+
* stable `code` the web client keys its copy on, the optional `subject`
107+
* naming what a safety policy objected to, and `retryable` so the UI can stop
108+
* inviting a retry that cannot succeed. Absent fields stay absent rather than
109+
* being defaulted — `retryable: false` invented here would tell a user a
110+
* transient outage is permanent.
111+
*/
112+
function mediaTaskErrorFromFailure(err: any): MediaTaskError {
113+
const subject = err?.subject;
114+
const retryable = err?.retryable;
115+
return {
116+
message: String(err && err.message ? err.message : err),
117+
status: typeof err?.status === 'number' ? err.status : 400,
118+
code: err?.code,
119+
...(subject === 'prompt' || subject === 'input_image' || subject === 'output_image'
120+
? { subject }
121+
: {}),
122+
...(typeof retryable === 'boolean' ? { retryable } : {}),
123+
};
124+
}
125+
99126
export function registerMediaRoutes(app: Express, ctx: RegisterMediaRoutesDeps) {
100127
const { db, design } = ctx;
101128
const { sendApiError, requireLocalDaemonRequest, isLocalSameOrigin, resolvedPortRef } = ctx.http;
@@ -295,11 +322,7 @@ export function registerMediaRoutes(app: Express, ctx: RegisterMediaRoutesDeps)
295322
})
296323
.catch((err: any) => {
297324
task.status = 'failed';
298-
task.error = {
299-
message: String(err && err.message ? err.message : err),
300-
status: typeof err?.status === 'number' ? err.status : 400,
301-
code: err?.code,
302-
};
325+
task.error = mediaTaskErrorFromFailure(err);
303326
task.endedAt = Date.now();
304327
persistMediaTask(task);
305328
if (analyticsContext && providerRequestSummary) {
@@ -335,11 +358,7 @@ export function registerMediaRoutes(app: Express, ctx: RegisterMediaRoutesDeps)
335358
} catch (err: any) {
336359
if (task) {
337360
task.status = 'failed';
338-
task.error = {
339-
message: String(err && err.message ? err.message : err),
340-
status: typeof err?.status === 'number' ? err.status : 400,
341-
code: err?.code,
342-
};
361+
task.error = mediaTaskErrorFromFailure(err);
343362
task.endedAt = Date.now();
344363
persistMediaTask(task);
345364
notifyTaskWaiters(task);

0 commit comments

Comments
 (0)