From 649c25d7d7e758a9d9a4df0585b92a11d4494587 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:22:56 +0800 Subject: [PATCH 01/24] chore(api): delete dead generation/types duplicates (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `domains/skills/generation/types/generation.ts` and `types/streaming.ts` re-declared `GeneratedSkill`, `SkillStreamEvent` and an unused `LlmOptions` but had zero importers — the generation service imports the canonical declarations from `shared/types/index.ts`. Removing them before #1242 extends `GeneratedSkill` (mode + references/assets) so the extension lands in exactly one place. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../skills/generation/types/generation.ts | 29 ------------------- .../skills/generation/types/streaming.ts | 7 ----- 2 files changed, 36 deletions(-) delete mode 100644 ornn-api/src/domains/skills/generation/types/generation.ts delete mode 100644 ornn-api/src/domains/skills/generation/types/streaming.ts diff --git a/ornn-api/src/domains/skills/generation/types/generation.ts b/ornn-api/src/domains/skills/generation/types/generation.ts deleted file mode 100644 index 874ced46..00000000 --- a/ornn-api/src/domains/skills/generation/types/generation.ts +++ /dev/null @@ -1,29 +0,0 @@ -/** Output shape from LLM skill generation. */ -export interface GeneratedSkill { - name: string; - description: string; - category: "plain" | "runtime-based"; - tags: string[]; - /** Markdown body content (no frontmatter — frontmatter built by client). */ - readmeBody: string; - runtimes: string[]; - /** Package dependencies required by this skill (npm for node, pip for python). */ - dependencies: string[]; - /** Environment variable names required by this skill. */ - envVars: string[]; - /** Script files to place in scripts/ directory. */ - scripts: Array<{ filename: string; content: string }>; - // exactOptionalPropertyTypes (#657): matches the Zod schema enum - // (`["text", "file"]`) — optional, so we widen with `| undefined`. - outputType?: "text" | "file" | undefined; -} - -/** Options for LLM completion calls. */ -export interface LlmOptions { - model?: string; - maxTokens?: number; - temperature?: number; - timeoutMs?: number; - /** System-level prompt sent as role: "system" before the user message. */ - systemPrompt?: string; -} diff --git a/ornn-api/src/domains/skills/generation/types/streaming.ts b/ornn-api/src/domains/skills/generation/types/streaming.ts deleted file mode 100644 index eea4b8d7..00000000 --- a/ornn-api/src/domains/skills/generation/types/streaming.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** Discriminated union of events emitted during streaming skill generation. */ -export type SkillStreamEvent = - | { type: "generation_start" } - | { type: "token"; content: string } - | { type: "generation_complete"; raw: string } - | { type: "validation_error"; message: string; retrying: boolean } - | { type: "error"; message: string }; From f3fa660b2452fcc04fe98889720a78ffe50fabd0 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:24:09 +0800 Subject: [PATCH 02/24] refactor(api): split generation route helpers out of routes.ts (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `routes.ts` was 548 lines — over the 500-line file limit — and mixed three concerns: request parsing, the SSE transport + quota reconcile, and ZIP-to-prompt context extraction. Pure move, no behaviour change: - `streaming.ts` now owns `preflight`, `resolveKeepAliveMs` and `streamGenerationEvents` (the #808/#827 ordering guarantees travel with their doc comments). - `packageContext.ts` now owns `analyzePackageContent`. `routes.ts` drops to 354 lines so the `mode` parsing in #1242 has room to land without breaching the limit again. The existing route tests exercise both helpers through the mounted routes and still pass. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../skills/generation/packageContext.ts | 64 ++++++ .../src/domains/skills/generation/routes.ts | 198 +----------------- .../domains/skills/generation/streaming.ts | 172 +++++++++++++++ 3 files changed, 238 insertions(+), 196 deletions(-) create mode 100644 ornn-api/src/domains/skills/generation/packageContext.ts create mode 100644 ornn-api/src/domains/skills/generation/streaming.ts diff --git a/ornn-api/src/domains/skills/generation/packageContext.ts b/ornn-api/src/domains/skills/generation/packageContext.ts new file mode 100644 index 00000000..355fd39b --- /dev/null +++ b/ornn-api/src/domains/skills/generation/packageContext.ts @@ -0,0 +1,64 @@ +/** + * Turns an uploaded skill package ZIP into the plain-text context block + * the multipart branch of `POST /skills/generate` prepends to the prompt. + * + * Split out of `routes.ts` (#1242). Only `SKILL.md` plus anything under + * `scripts/`, `references/` and `assets/` is read — the same folders the + * upload validator allows at the package root. + * + * @module domains/skills/generation/packageContext + */ + +import JSZip from "jszip"; +import { resolveZipRoot } from "../../../shared/utils/zip"; +import { createLogger } from "../../../shared/logger"; + +const logger = createLogger("skillGenerationPackageContext"); + +const RELEVANT_FILES = ["SKILL.md"]; +const RELEVANT_DIRS = ["scripts/", "references/", "assets/"]; + +/** + * Read content from a ZIP package for analysis. + */ +export async function analyzePackageContent(zipBuffer: Uint8Array): Promise { + const zip = await JSZip.loadAsync(zipBuffer); + const allPaths = Object.keys(zip.files); + resolveZipRoot(zip, allPaths); + const parts: string[] = []; + + for (const path of allPaths) { + const file = zip.files[path]; + // allPaths is `Object.keys(zip.files)`, but noUncheckedIndexedAccess + // (#450) widens the lookup to `T | undefined`. Defensive skip. + if (!file || file.dir) continue; + + // Check if this is a relevant file + const segments = path.split("/").filter(Boolean); + let relativePath = path; + if (segments.length > 1) { + const firstEntry = segments[0]!; + const folderEntry = zip.files[firstEntry + "/"]; + if (folderEntry && folderEntry.dir) { + relativePath = segments.slice(1).join("/"); + } + } + + const isRelevant = RELEVANT_FILES.includes(relativePath) || + RELEVANT_DIRS.some((d) => relativePath.startsWith(d)); + + if (isRelevant) { + try { + const content = await file.async("string"); + parts.push(`--- ${relativePath} ---\n${content}`); + } catch (err) { + // Skip binary or unreadable files. Log so an upload that's + // 100% binary doesn't silently produce an empty generation + // context (#579). + logger.debug({ err, relativePath }, "generation: skipping unreadable file"); + } + } + } + + return parts.join("\n\n"); +} diff --git a/ornn-api/src/domains/skills/generation/routes.ts b/ornn-api/src/domains/skills/generation/routes.ts index 296e3ccd..508584c2 100644 --- a/ornn-api/src/domains/skills/generation/routes.ts +++ b/ornn-api/src/domains/skills/generation/routes.ts @@ -5,14 +5,9 @@ */ import { Hono } from "hono"; -import { streamSSE } from "hono/streaming"; -import type { Context } from "hono"; import type { SkillGenerationService } from "./service"; import type { QuotaService } from "../../quota/service"; import type { LlmProvidersService } from "../../settings/llmProviders/service"; -import { throwQuotaError } from "../../quota/routes"; -import { throwModelResolutionError } from "../../settings/llmProviders/routes"; -import type { ChargeOutcome } from "../../quota/types"; import { type AuthVariables, nyxidAuthMiddleware, @@ -20,11 +15,11 @@ import { getAuth, } from "../../../middleware/nyxidAuth"; import { AppError } from "../../../shared/types/index"; -import { resolveZipRoot } from "../../../shared/utils/zip"; import { validateBody, getValidatedBody } from "../../../middleware/validate"; import { rateLimit } from "../../../middleware/rateLimit"; import { fetchGithubSourceBundle } from "./githubFetcher"; -import JSZip from "jszip"; +import { analyzePackageContent } from "./packageContext"; +import { preflight, resolveKeepAliveMs, streamGenerationEvents } from "./streaming"; import { createLogger } from "../../../shared/logger"; import { z } from "zod"; @@ -52,195 +47,6 @@ export interface GenerationRoutesConfig { llmProvidersService: LlmProvidersService; } -/** Helper to resolve keep-alive ms with a safe fallback. */ -async function resolveKeepAliveMs( - resolver: () => Promise, -): Promise { - try { - const v = await resolver(); - return Number.isFinite(v) && v > 0 ? v : 15_000; - } catch (err) { - logger.warn( - { err: (err as Error).message }, - "Failed to resolve skillGen sseKeepAliveMs; using 15s default", - ); - return 15_000; - } -} - -/** - * Run model resolution + quota reserve for a skill-gen request. Returns - * the resolved model id; throws the appropriate AppError when either - * gate fails (models → 503/4xx, quota → 429). - * - * Order is load-bearing (#808): model resolution runs FIRST so a - * resolution failure can't strand a reserved quota slot. `resolveModel` - * is a pure catalog read (no LLM), so reserving last still keeps the - * "429 before any LLM cost" guarantee. Once `checkAllowed` reserves, - * every caller threads the result straight into `streamGenerationEvents`, - * whose `finally` always reconciles the reservation (commit on success, - * release on system_error/abort). - */ -async function preflight( - c: Context<{ Variables: AuthVariables }>, - quotaService: QuotaService, - llmProvidersService: LlmProvidersService, - requestedModelId: string | undefined, -): Promise<{ - modelId: string; - userId: string; - permissions: readonly string[] | undefined; - reservedAt: Date; -}> { - const authCtx = getAuth(c); - - const resolution = await llmProvidersService.resolveModel({ - surface: "skillGen", - // exactOptionalPropertyTypes (#657) - ...(requestedModelId !== undefined ? { requested: requestedModelId } : {}), - }); - if (resolution.kind !== "ok") throwModelResolutionError(resolution); - - // Capture the reservation instant so the charge lands in the SAME - // month bucket the slot was reserved against (#827) — see the - // playground route for the boundary-straddle rationale. - const reservedAt = new Date(); - const decision = await quotaService.checkAllowed({ - userId: authCtx.userId, - permissions: authCtx.permissions, - surface: "skillGen", - now: reservedAt, - }); - if (!decision.allowed) throwQuotaError(decision); - - return { - modelId: resolution.modelId, - userId: authCtx.userId, - permissions: authCtx.permissions, - reservedAt, - }; -} - -/** - * Stream generation events via SSE with keep-alive. When `chargeAfter` - * is set, fires a quota charge after the stream finishes — outcome - * derived from whether the stream emitted a `generation_complete` event - * (skill-side success), a `validation_error` (skill ran but produced - * invalid output — still chargeable), or only `error` events - * (system_error — no charge). - */ -async function streamGenerationEvents( - c: Context, - events: AsyncIterable<{ type: string; [key: string]: unknown }>, - keepAliveIntervalMs: number, - chargeAfter?: { - quotaService: QuotaService; - userId: string; - permissions: readonly string[] | undefined; - /** Resolved model id used for the LLM call — flows into `usedByModel`. */ - modelId: string; - /** - * Reservation instant captured at `preflight` time (#827). Threaded - * into `chargeOnCompletion` as `now` so the commit/release reconciles - * against the month bucket the slot was reserved in, not wall-clock. - */ - reservedAt: Date; - }, -) { - c.header("Cache-Control", "no-cache"); - c.header("Connection", "keep-alive"); - c.header("X-Accel-Buffering", "no"); - - return streamSSE(c, async (stream) => { - const keepAlive = setInterval(() => { - stream.writeSSE({ data: "", event: "keepalive" }).catch(() => {}); - }, keepAliveIntervalMs); - - const signal = c.req.raw.signal; - const onAbort = () => clearInterval(keepAlive); - signal.addEventListener("abort", onAbort, { once: true }); - - let outcome: ChargeOutcome = "system_error"; - - try { - for await (const event of events) { - await stream.writeSSE({ data: JSON.stringify(event) }); - if (event.type === "generation_complete") outcome = "success"; - else if (event.type === "validation_error") outcome = "skill_error"; - } - } finally { - clearInterval(keepAlive); - signal.removeEventListener("abort", onAbort); - if (chargeAfter) { - await chargeAfter.quotaService - .chargeOnCompletion({ - userId: chargeAfter.userId, - permissions: chargeAfter.permissions, - surface: "skillGen", - outcome, - modelId: chargeAfter.modelId, - // Reconcile against the reserved month bucket (#827). - now: chargeAfter.reservedAt, - }) - .catch((err) => { - logger.warn( - { userId: chargeAfter.userId, err: (err as Error).message }, - "Quota charge after skill-gen stream failed", - ); - }); - } - } - }); -} - -/** - * Read content from a ZIP package for analysis. - */ -async function analyzePackageContent(zipBuffer: Uint8Array): Promise { - const zip = await JSZip.loadAsync(zipBuffer); - const allPaths = Object.keys(zip.files); - resolveZipRoot(zip, allPaths); - const parts: string[] = []; - - const relevantFiles = ["SKILL.md"]; - const relevantDirs = ["scripts/", "references/", "assets/"]; - - for (const path of allPaths) { - const file = zip.files[path]; - // allPaths is `Object.keys(zip.files)`, but noUncheckedIndexedAccess - // (#450) widens the lookup to `T | undefined`. Defensive skip. - if (!file || file.dir) continue; - - // Check if this is a relevant file - const segments = path.split("/").filter(Boolean); - let relativePath = path; - if (segments.length > 1) { - const firstEntry = segments[0]!; - const folderEntry = zip.files[firstEntry + "/"]; - if (folderEntry && folderEntry.dir) { - relativePath = segments.slice(1).join("/"); - } - } - - const isRelevant = relevantFiles.includes(relativePath) || - relevantDirs.some((d) => relativePath.startsWith(d)); - - if (isRelevant) { - try { - const content = await file.async("string"); - parts.push(`--- ${relativePath} ---\n${content}`); - } catch (err) { - // Skip binary or unreadable files. Log so an upload that's - // 100% binary doesn't silently produce an empty generation - // context (#579). - logger.debug({ err, relativePath }, "generation: skipping unreadable file"); - } - } - } - - return parts.join("\n\n"); -} - export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ Variables: AuthVariables }> { const { generationService, keepAliveIntervalMsResolver, quotaService, llmProvidersService } = config; const app = new Hono<{ Variables: AuthVariables }>(); diff --git a/ornn-api/src/domains/skills/generation/streaming.ts b/ornn-api/src/domains/skills/generation/streaming.ts new file mode 100644 index 00000000..e37d09e9 --- /dev/null +++ b/ornn-api/src/domains/skills/generation/streaming.ts @@ -0,0 +1,172 @@ +/** + * SSE transport + pre-stream gates shared by every skill-generation + * route: model resolution → quota reserve (`preflight`), keep-alive + * resolution, and the event pump that writes frames and reconciles the + * quota charge when the stream ends (`streamGenerationEvents`). + * + * Split out of `routes.ts` (#1242) so the route module only owns request + * parsing; the ordering guarantees documented here (#808/#827) are + * unchanged. + * + * @module domains/skills/generation/streaming + */ + +import { streamSSE } from "hono/streaming"; +import type { Context } from "hono"; +import type { QuotaService } from "../../quota/service"; +import type { LlmProvidersService } from "../../settings/llmProviders/service"; +import { throwQuotaError } from "../../quota/routes"; +import { throwModelResolutionError } from "../../settings/llmProviders/routes"; +import type { ChargeOutcome } from "../../quota/types"; +import { type AuthVariables, getAuth } from "../../../middleware/nyxidAuth"; +import { createLogger } from "../../../shared/logger"; + +const logger = createLogger("skillGenerationStreaming"); + +/** Keep-alive cadence used when the admin setting cannot be resolved. */ +const FALLBACK_KEEP_ALIVE_MS = 15_000; + +/** Helper to resolve keep-alive ms with a safe fallback. */ +export async function resolveKeepAliveMs( + resolver: () => Promise, +): Promise { + try { + const v = await resolver(); + return Number.isFinite(v) && v > 0 ? v : FALLBACK_KEEP_ALIVE_MS; + } catch (err) { + logger.warn( + { err: (err as Error).message }, + "Failed to resolve skillGen sseKeepAliveMs; using 15s default", + ); + return FALLBACK_KEEP_ALIVE_MS; + } +} + +export interface PreflightResult { + modelId: string; + userId: string; + permissions: readonly string[] | undefined; + reservedAt: Date; +} + +/** + * Run model resolution + quota reserve for a skill-gen request. Returns + * the resolved model id; throws the appropriate AppError when either + * gate fails (models → 503/4xx, quota → 429). + * + * Order is load-bearing (#808): model resolution runs FIRST so a + * resolution failure can't strand a reserved quota slot. `resolveModel` + * is a pure catalog read (no LLM), so reserving last still keeps the + * "429 before any LLM cost" guarantee. Once `checkAllowed` reserves, + * every caller threads the result straight into `streamGenerationEvents`, + * whose `finally` always reconciles the reservation (commit on success, + * release on system_error/abort). + */ +export async function preflight( + c: Context<{ Variables: AuthVariables }>, + quotaService: QuotaService, + llmProvidersService: LlmProvidersService, + requestedModelId: string | undefined, +): Promise { + const authCtx = getAuth(c); + + const resolution = await llmProvidersService.resolveModel({ + surface: "skillGen", + // exactOptionalPropertyTypes (#657) + ...(requestedModelId !== undefined ? { requested: requestedModelId } : {}), + }); + if (resolution.kind !== "ok") throwModelResolutionError(resolution); + + // Capture the reservation instant so the charge lands in the SAME + // month bucket the slot was reserved against (#827) — see the + // playground route for the boundary-straddle rationale. + const reservedAt = new Date(); + const decision = await quotaService.checkAllowed({ + userId: authCtx.userId, + permissions: authCtx.permissions, + surface: "skillGen", + now: reservedAt, + }); + if (!decision.allowed) throwQuotaError(decision); + + return { + modelId: resolution.modelId, + userId: authCtx.userId, + permissions: authCtx.permissions, + reservedAt, + }; +} + +export interface ChargeAfter { + quotaService: QuotaService; + userId: string; + permissions: readonly string[] | undefined; + /** Resolved model id used for the LLM call — flows into `usedByModel`. */ + modelId: string; + /** + * Reservation instant captured at `preflight` time (#827). Threaded + * into `chargeOnCompletion` as `now` so the commit/release reconciles + * against the month bucket the slot was reserved in, not wall-clock. + */ + reservedAt: Date; +} + +/** + * Stream generation events via SSE with keep-alive. When `chargeAfter` + * is set, fires a quota charge after the stream finishes — outcome + * derived from whether the stream emitted a `generation_complete` event + * (skill-side success), a `validation_error` (skill ran but produced + * invalid output — still chargeable), or only `error` events + * (system_error — no charge). + */ +export async function streamGenerationEvents( + c: Context, + events: AsyncIterable<{ type: string; [key: string]: unknown }>, + keepAliveIntervalMs: number, + chargeAfter?: ChargeAfter, +) { + c.header("Cache-Control", "no-cache"); + c.header("Connection", "keep-alive"); + c.header("X-Accel-Buffering", "no"); + + return streamSSE(c, async (stream) => { + const keepAlive = setInterval(() => { + stream.writeSSE({ data: "", event: "keepalive" }).catch(() => {}); + }, keepAliveIntervalMs); + + const signal = c.req.raw.signal; + const onAbort = () => clearInterval(keepAlive); + signal.addEventListener("abort", onAbort, { once: true }); + + let outcome: ChargeOutcome = "system_error"; + + try { + for await (const event of events) { + await stream.writeSSE({ data: JSON.stringify(event) }); + if (event.type === "generation_complete") outcome = "success"; + else if (event.type === "validation_error") outcome = "skill_error"; + } + } finally { + clearInterval(keepAlive); + signal.removeEventListener("abort", onAbort); + if (chargeAfter) { + await chargeAfter.quotaService + .chargeOnCompletion({ + userId: chargeAfter.userId, + permissions: chargeAfter.permissions, + surface: "skillGen", + outcome, + modelId: chargeAfter.modelId, + // Reconcile against the reserved month bucket (#827). + now: chargeAfter.reservedAt, + }) + .catch((err) => { + logger.warn( + { userId: chargeAfter.userId, err: (err as Error).message }, + "Quota charge after skill-gen stream failed", + ); + }); + } + } + }); +} From 89774f91ab89748820e01fc38412bc735f9ab356 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:25:39 +0800 Subject: [PATCH 03/24] refactor(api): extract shared LLM stream loop in generation svc (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four `generate*` generators each carried an identical 60-line preamble + stream loop (resolve defaults → abort check → `generation_start` → pump tokens → map abort/provider failures to an `error` frame). That duplication is what would have pushed `service.ts` past the 500-line limit once mode handling lands in #1242, and it meant any fix to the loop had to be applied four times. - `begin()` owns the preamble and `streamLlm()` owns the token pump; both are `AsyncGenerator`s consumed with `yield*` so the emitted event sequence is byte-for-byte what each generator produced before (the existing per-generator log labels are threaded through). - `completeLlm()` owns the non-streaming call the single-turn retry uses. - `generatedSkillSchema` + the parse/clean/validate routine move to `validation.ts` (`parseGeneratedSkill`); `parseAndValidate` stays on the service as a thin delegate because the tests call it directly. No behaviour change — the 82 generation tests pass unmodified. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../src/domains/skills/generation/service.ts | 322 ++++++------------ .../domains/skills/generation/validation.ts | 77 +++++ 2 files changed, 177 insertions(+), 222 deletions(-) create mode 100644 ornn-api/src/domains/skills/generation/validation.ts diff --git a/ornn-api/src/domains/skills/generation/service.ts b/ornn-api/src/domains/skills/generation/service.ts index d2ca1e49..866a3bfe 100644 --- a/ornn-api/src/domains/skills/generation/service.ts +++ b/ornn-api/src/domains/skills/generation/service.ts @@ -5,7 +5,6 @@ * @module domains/skills/generation/service */ -import { z } from "zod"; import type { NyxLlmClient, ResponsesApiStreamEvent, ResponsesApiInputMessage } from "../../../clients/nyxid/llm"; import type { GeneratedSkill, SkillStreamEvent } from "../../../shared/types/index"; import { @@ -16,27 +15,10 @@ import { OPENAPI_GENERATION_SYSTEM_PROMPT, SOURCE_CODE_GENERATION_SYSTEM_PROMPT, } from "./prompts"; +import { parseGeneratedSkill } from "./validation"; import { createLogger } from "../../../shared/logger"; const logger = createLogger("skillGenerationService"); -const generatedSkillSchema = z.object({ - name: z.string().min(1).max(100).regex(/^[a-z0-9-]+$/), - description: z.string().min(10).max(500), - category: z.enum(["plain", "runtime-based"]), - outputType: z.enum(["text", "file"]).optional(), - tags: z.array(z.string().min(2).max(30).regex(/^[a-z0-9-]+$/)).min(1).max(10), - readmeBody: z.string().min(50).max(20_000), - runtimes: z.array(z.string()).default([]), - dependencies: z.array(z.string().max(200)).default([]), - envVars: z.array(z.string().max(100)).default([]), - scripts: z.array(z.object({ - filename: z.string().min(1).max(200), - content: z.string().min(1).max(50_000), - })).default([]), -}); - -export { generatedSkillSchema }; - /** * Per-call resolution of LLM defaults from admin settings (`skillGen` * section + selected provider's `maxOutputTokens` / `defaultTemperature`). @@ -61,6 +43,12 @@ export interface GenerationServiceConfig { defaultsResolver: SkillGenLlmDefaultsResolver; } +/** Resolved per-call LLM parameters shared by every generator. */ +interface LlmCallContext { + model: string; + defaults: SkillGenLlmDefaults; +} + export class SkillGenerationService { private readonly llmClient: NyxLlmClient; private readonly defaultsResolver: SkillGenLlmDefaultsResolver; @@ -81,51 +69,58 @@ export class SkillGenerationService { } /** - * Direct generation streaming. Streams tokens via SSE events. - * Uses Nyx Provider Responses API format. `modelOverride` (when set) - * picks an admin-curated model; otherwise the service-level default - * applies. + * Common preamble for every generator: resolve LLM defaults, honour a + * pre-aborted signal, and open the stream with `generation_start`. + * Returns `null` after yielding the terminal `error` event so callers + * can simply `return`. */ - async *generateStream( - query: string, - signal?: AbortSignal, - modelOverride?: string, - ): AsyncIterable { + private async *begin( + signal: AbortSignal | undefined, + modelOverride: string | undefined, + ): AsyncGenerator { let defaults: SkillGenLlmDefaults; try { defaults = await this.resolveDefaults(); } catch (err) { yield { type: "error", message: (err as Error).message }; - return; + return null; } const model = modelOverride ?? defaults.model; if (signal?.aborted) { yield { type: "error", message: "Request aborted" }; - return; + return null; } yield { type: "generation_start" }; + return { model, defaults }; + } - const { userPrompt } = buildDirectGenerationPrompt(query); - const input: ResponsesApiInputMessage[] = [ - { role: "developer", content: GENERATION_SYSTEM_PROMPT }, - { role: "user", content: userPrompt }, - ]; - + /** + * Stream one LLM call, yielding `token` events as text arrives. + * Returns the accumulated text, or `null` after yielding the terminal + * `error` event (abort mid-stream or provider failure). `logLabel` + * keeps the per-generator error log lines distinguishable. + */ + private async *streamLlm( + input: ResponsesApiInputMessage[], + ctx: LlmCallContext, + signal: AbortSignal | undefined, + logLabel: string, + ): AsyncGenerator { let accumulated = ""; try { const streamEvents = this.llmClient.stream({ - model, + model: ctx.model, input, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, + max_output_tokens: ctx.defaults.maxOutputTokens, + temperature: ctx.defaults.temperature, }); for await (const event of streamEvents) { if (signal?.aborted) { yield { type: "error", message: "Request aborted" }; - return; + return null; } const text = extractTextFromEvent(event); @@ -136,11 +131,60 @@ export class SkillGenerationService { } } catch (err) { const message = err instanceof Error ? err.message : String(err); - logger.error({ err: message }, "LLM stream error"); + logger.error({ err: message }, `${logLabel} LLM stream error`); yield { type: "error", message: `LLM error: ${message}` }; - return; + return null; } + return accumulated; + } + + /** Non-streaming completion — used for the single-turn retry. */ + private async completeLlm( + input: ResponsesApiInputMessage[], + ctx: LlmCallContext, + ): Promise { + const outputs = await this.llmClient.complete({ + model: ctx.model, + input, + max_output_tokens: ctx.defaults.maxOutputTokens, + temperature: ctx.defaults.temperature, + }); + + let text = ""; + for (const output of outputs) { + if (output.content) { + for (const part of output.content) { + if (part.text) text += part.text; + } + } + } + return text; + } + + /** + * Direct generation streaming. Streams tokens via SSE events. + * Uses Nyx Provider Responses API format. `modelOverride` (when set) + * picks an admin-curated model; otherwise the service-level default + * applies. + */ + async *generateStream( + query: string, + signal?: AbortSignal, + modelOverride?: string, + ): AsyncIterable { + const ctx = yield* this.begin(signal, modelOverride); + if (!ctx) return; + + const { userPrompt } = buildDirectGenerationPrompt(query); + const input: ResponsesApiInputMessage[] = [ + { role: "developer", content: GENERATION_SYSTEM_PROMPT }, + { role: "user", content: userPrompt }, + ]; + + const accumulated = yield* this.streamLlm(input, ctx, signal, "direct"); + if (accumulated === null) return; + // Validate the accumulated output const parsed = this.parseAndValidate(accumulated); if (!parsed) { @@ -155,21 +199,7 @@ export class SkillGenerationService { { role: "user", content: `${userPrompt}\n\nIMPORTANT: Output ONLY valid JSON. No markdown fences. No extra text.` }, ]; - const outputs = await this.llmClient.complete({ - model, - input: retryInput, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, - }); - - let retryText = ""; - for (const output of outputs) { - if (output.content) { - for (const part of output.content) { - if (part.text) retryText += part.text; - } - } - } + const retryText = await this.completeLlm(retryInput, ctx); const retryParsed = this.parseAndValidate(retryText); if (retryParsed) { @@ -199,20 +229,8 @@ export class SkillGenerationService { signal?: AbortSignal, modelOverride?: string, ): AsyncIterable { - let defaults: SkillGenLlmDefaults; - try { - defaults = await this.resolveDefaults(); - } catch (err) { - yield { type: "error", message: (err as Error).message }; - return; - } - const model = modelOverride ?? defaults.model; - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - yield { type: "generation_start" }; + const ctx = yield* this.begin(signal, modelOverride); + if (!ctx) return; // Put system prompt as developer message in input array (not as instructions) // because some LLM providers ignore the instructions field. @@ -232,34 +250,8 @@ export class SkillGenerationService { }), ]; - let accumulated = ""; - - try { - const streamEvents = this.llmClient.stream({ - model, - input, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, - }); - - for await (const event of streamEvents) { - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - const text = extractTextFromEvent(event); - if (text) { - accumulated += text; - yield { type: "token", content: text }; - } - } - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - logger.error({ err: message }, "LLM multi-turn stream error"); - yield { type: "error", message: `LLM error: ${message}` }; - return; - } + const accumulated = yield* this.streamLlm(input, ctx, signal, "multi-turn"); + if (accumulated === null) return; logger.info( { accumulatedLength: accumulated.length, first200: accumulated.slice(0, 200), last200: accumulated.slice(-200) }, @@ -287,20 +279,8 @@ export class SkillGenerationService { signal?: AbortSignal, modelOverride?: string, ): AsyncIterable { - let defaults: SkillGenLlmDefaults; - try { - defaults = await this.resolveDefaults(); - } catch (err) { - yield { type: "error", message: (err as Error).message }; - return; - } - const model = modelOverride ?? defaults.model; - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - yield { type: "generation_start" }; + const ctx = yield* this.begin(signal, modelOverride); + if (!ctx) return; const userPrompt = buildOpenApiGenerationPrompt(specContent, options); const input: ResponsesApiInputMessage[] = [ @@ -308,34 +288,8 @@ export class SkillGenerationService { { role: "user", content: userPrompt }, ]; - let accumulated = ""; - - try { - const streamEvents = this.llmClient.stream({ - model, - input, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, - }); - - for await (const event of streamEvents) { - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - const text = extractTextFromEvent(event); - if (text) { - accumulated += text; - yield { type: "token", content: text }; - } - } - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - logger.error({ err: message }, "OpenAPI generation LLM stream error"); - yield { type: "error", message: `LLM error: ${message}` }; - return; - } + const accumulated = yield* this.streamLlm(input, ctx, signal, "OpenAPI generation"); + if (accumulated === null) return; const parsed = this.parseAndValidate(accumulated); if (!parsed) { @@ -366,20 +320,8 @@ export class SkillGenerationService { signal?: AbortSignal, modelOverride?: string, ): AsyncIterable { - let defaults: SkillGenLlmDefaults; - try { - defaults = await this.resolveDefaults(); - } catch (err) { - yield { type: "error", message: (err as Error).message }; - return; - } - const model = modelOverride ?? defaults.model; - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - yield { type: "generation_start" }; + const ctx = yield* this.begin(signal, modelOverride); + if (!ctx) return; const userPrompt = buildSourceCodeGenerationPrompt(code, options); const input: ResponsesApiInputMessage[] = [ @@ -387,34 +329,8 @@ export class SkillGenerationService { { role: "user", content: userPrompt }, ]; - let accumulated = ""; - - try { - const streamEvents = this.llmClient.stream({ - model, - input, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, - }); - - for await (const event of streamEvents) { - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } - - const text = extractTextFromEvent(event); - if (text) { - accumulated += text; - yield { type: "token", content: text }; - } - } - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - logger.error({ err: message }, "Source-code generation LLM stream error"); - yield { type: "error", message: `LLM error: ${message}` }; - return; - } + const accumulated = yield* this.streamLlm(input, ctx, signal, "Source-code generation"); + if (accumulated === null) return; const parsed = this.parseAndValidate(accumulated); if (!parsed) { @@ -426,45 +342,7 @@ export class SkillGenerationService { } parseAndValidate(raw: string): GeneratedSkill | null { - try { - let cleaned = raw.replace(/```json\n?/g, "").replace(/```\n?/g, "").trim(); - - const jsonStart = cleaned.indexOf("{"); - const jsonEnd = cleaned.lastIndexOf("}"); - if (jsonStart >= 0 && jsonEnd > jsonStart) { - cleaned = cleaned.slice(jsonStart, jsonEnd + 1); - } - - const json = JSON.parse(cleaned); - - // Handle backward-compat: rename readmeMd -> readmeBody - if (json.readmeMd && !json.readmeBody) { - const md = json.readmeMd as string; - const fmEnd = md.indexOf("\n---", 3); - json.readmeBody = fmEnd > 0 ? md.slice(fmEnd + 4).trim() : md; - delete json.readmeMd; - } - - const result = generatedSkillSchema.safeParse(json); - if (!result.success) { - logger.debug({ errors: result.error.issues }, "Generated skill validation failed"); - return null; - } - - // The Zod-inferred shape and GeneratedSkill match in spirit but - // Zod surfaces `outputType` as `"text" | "file" | undefined` - // (explicit undefined, not optional) which exactOptionalPropertyTypes - // (#657) treats as different from the interface's `outputType?:`. - // Same runtime shape; cast is safe. - return result.data as GeneratedSkill; - } catch (err) { - // Generated-skill JSON parse failed. Caller treats null as - // "regenerate" or "give up" depending on retry budget. Logging - // so we can spot a model that's consistently producing - // unparseable output (#579). - logger.debug({ err }, "generated skill JSON parse failed"); - return null; - } + return parseGeneratedSkill(raw); } } diff --git a/ornn-api/src/domains/skills/generation/validation.ts b/ornn-api/src/domains/skills/generation/validation.ts new file mode 100644 index 00000000..d64b2b08 --- /dev/null +++ b/ornn-api/src/domains/skills/generation/validation.ts @@ -0,0 +1,77 @@ +/** + * Parsing + schema validation of the JSON document the LLM returns for + * a generated skill. Split out of `service.ts` (#1242) so the schema has + * one home and the service only orchestrates streaming. + * + * @module domains/skills/generation/validation + */ + +import { z } from "zod"; +import type { GeneratedSkill } from "../../../shared/types/index"; +import { createLogger } from "../../../shared/logger"; + +const logger = createLogger("skillGenerationValidation"); + +export const generatedSkillSchema = z.object({ + name: z.string().min(1).max(100).regex(/^[a-z0-9-]+$/), + description: z.string().min(10).max(500), + category: z.enum(["plain", "runtime-based"]), + outputType: z.enum(["text", "file"]).optional(), + tags: z.array(z.string().min(2).max(30).regex(/^[a-z0-9-]+$/)).min(1).max(10), + readmeBody: z.string().min(50).max(20_000), + runtimes: z.array(z.string()).default([]), + dependencies: z.array(z.string().max(200)).default([]), + envVars: z.array(z.string().max(100)).default([]), + scripts: z.array(z.object({ + filename: z.string().min(1).max(200), + content: z.string().min(1).max(50_000), + })).default([]), +}); + +/** + * Strip markdown fences / surrounding prose, parse the JSON object and + * validate it against {@link generatedSkillSchema}. Returns `null` when + * the text is not a schema-valid skill document; callers decide whether + * to retry or give up. + */ +export function parseGeneratedSkill(raw: string): GeneratedSkill | null { + try { + let cleaned = raw.replace(/```json\n?/g, "").replace(/```\n?/g, "").trim(); + + const jsonStart = cleaned.indexOf("{"); + const jsonEnd = cleaned.lastIndexOf("}"); + if (jsonStart >= 0 && jsonEnd > jsonStart) { + cleaned = cleaned.slice(jsonStart, jsonEnd + 1); + } + + const json = JSON.parse(cleaned); + + // Handle backward-compat: rename readmeMd -> readmeBody + if (json.readmeMd && !json.readmeBody) { + const md = json.readmeMd as string; + const fmEnd = md.indexOf("\n---", 3); + json.readmeBody = fmEnd > 0 ? md.slice(fmEnd + 4).trim() : md; + delete json.readmeMd; + } + + const result = generatedSkillSchema.safeParse(json); + if (!result.success) { + logger.debug({ errors: result.error.issues }, "Generated skill validation failed"); + return null; + } + + // The Zod-inferred shape and GeneratedSkill match in spirit but + // Zod surfaces `outputType` as `"text" | "file" | undefined` + // (explicit undefined, not optional) which exactOptionalPropertyTypes + // (#657) treats as different from the interface's `outputType?:`. + // Same runtime shape; cast is safe. + return result.data as GeneratedSkill; + } catch (err) { + // Generated-skill JSON parse failed. Caller treats null as + // "regenerate" or "give up" depending on retry budget. Logging + // so we can spot a model that's consistently producing + // unparseable output (#579). + logger.debug({ err }, "generated skill JSON parse failed"); + return null; + } +} From b672df07ad494db3e6fc67eade76c0758b5afee2 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:26:36 +0800 Subject: [PATCH 04/24] feat(api): add GenerationMode + references/assets to skill types (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract root for skill-generation modes: - `GENERATION_MODES` / `GenerationMode` (`"simple" | "advanced"`) and `DEFAULT_GENERATION_MODE = "advanced"`. Defaulting to `advanced` keeps every existing caller — agents already integrated against `POST /skills/generate` — on exactly the behaviour they had before modes existed. - `GeneratedSkill` gains `references` and `assets` (same `{ filename, content }` shape as `scripts`), the advanced-mode materials the skill package format already allows at the root but the generator never produced. The Zod schema defaults both to `[]` so older model output and the integration fixture (which omits the array fields) still validate; the JSON contract is text-only, so binary assets remain an upload-only concern. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/skills/generation/service.test.ts | 29 +++++++++++++++++ .../domains/skills/generation/validation.ts | 19 ++++++++--- ornn-api/src/shared/types/index.ts | 32 ++++++++++++++++++- 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/ornn-api/src/domains/skills/generation/service.test.ts b/ornn-api/src/domains/skills/generation/service.test.ts index d420266b..f522b9e5 100644 --- a/ornn-api/src/domains/skills/generation/service.test.ts +++ b/ornn-api/src/domains/skills/generation/service.test.ts @@ -677,4 +677,33 @@ describe("parseAndValidate", () => { test("non-JSON input returns null", () => { expect(svc().parseAndValidate("this is not json at all")).toBeNull(); }); + + test("references / assets default to [] when the model omits them (#1242)", () => { + const out = svc().parseAndValidate(VALID_SKILL); + expect(out).not.toBeNull(); + expect(out!.references).toEqual([]); + expect(out!.assets).toEqual([]); + }); + + test("references / assets pass through when the model emits them (#1242)", () => { + const withExtras = JSON.stringify({ + ...JSON.parse(VALID_SKILL), + references: [{ filename: "api.md", content: "# API\n\nReference material." }], + assets: [{ filename: "template.json", content: "{\"greeting\":\"hi\"}" }], + }); + const out = svc().parseAndValidate(withExtras); + expect(out).not.toBeNull(); + expect(out!.references).toEqual([ + { filename: "api.md", content: "# API\n\nReference material." }, + ]); + expect(out!.assets[0]!.filename).toBe("template.json"); + }); + + test("a references entry with empty content fails the schema (#1242)", () => { + const bad = JSON.stringify({ + ...JSON.parse(VALID_SKILL), + references: [{ filename: "empty.md", content: "" }], + }); + expect(svc().parseAndValidate(bad)).toBeNull(); + }); }); diff --git a/ornn-api/src/domains/skills/generation/validation.ts b/ornn-api/src/domains/skills/generation/validation.ts index d64b2b08..60e39777 100644 --- a/ornn-api/src/domains/skills/generation/validation.ts +++ b/ornn-api/src/domains/skills/generation/validation.ts @@ -12,6 +12,16 @@ import { createLogger } from "../../../shared/logger"; const logger = createLogger("skillGenerationValidation"); +/** + * One emitted package file. Shared by `scripts`, `references` and + * `assets` — the JSON contract can only carry text, so binary assets are + * out of scope for generation (they still arrive via upload). + */ +const generatedFileSchema = z.object({ + filename: z.string().min(1).max(200), + content: z.string().min(1).max(50_000), +}); + export const generatedSkillSchema = z.object({ name: z.string().min(1).max(100).regex(/^[a-z0-9-]+$/), description: z.string().min(10).max(500), @@ -22,10 +32,11 @@ export const generatedSkillSchema = z.object({ runtimes: z.array(z.string()).default([]), dependencies: z.array(z.string().max(200)).default([]), envVars: z.array(z.string().max(100)).default([]), - scripts: z.array(z.object({ - filename: z.string().min(1).max(200), - content: z.string().min(1).max(50_000), - })).default([]), + scripts: z.array(generatedFileSchema).default([]), + // Advanced-mode extras (#1242). Defaulted so older model output (and + // the integration fixtures) that omit them still validate. + references: z.array(generatedFileSchema).default([]), + assets: z.array(generatedFileSchema).default([]), }); /** diff --git a/ornn-api/src/shared/types/index.ts b/ornn-api/src/shared/types/index.ts index 409d6245..0dd83e4c 100644 --- a/ornn-api/src/shared/types/index.ts +++ b/ornn-api/src/shared/types/index.ts @@ -564,6 +564,31 @@ export interface TagDocument { // Generation // --------------------------------------------------------------------------- +/** + * Caller-chosen package shape for `POST /skills/generate` (#1242). + * + * - `simple` — the package is `SKILL.md` only. The model is told not to + * emit scripts / references / assets and the server + * rejects output that carries any. + * - `advanced` — the package may carry `scripts/`, `references/` and + * `assets/` alongside `SKILL.md`. + */ +export const GENERATION_MODES = ["simple", "advanced"] as const; +export type GenerationMode = (typeof GENERATION_MODES)[number]; + +/** + * Applied when the caller omits `mode`. `advanced` is what every caller + * got before modes existed, so omitting the field stays backward + * compatible for agents already integrated against the endpoint. + */ +export const DEFAULT_GENERATION_MODE: GenerationMode = "advanced"; + +/** One text file the model emits for `scripts/`, `references/` or `assets/`. */ +export interface GeneratedSkillFile { + filename: string; + content: string; +} + export interface GeneratedSkill { name: string; description: string; @@ -574,7 +599,12 @@ export interface GeneratedSkill { runtimes: string[]; dependencies: string[]; envVars: string[]; - scripts: Array<{ filename: string; content: string }>; + /** Files to place under `scripts/`. Always empty in `simple` mode. */ + scripts: GeneratedSkillFile[]; + /** Files to place under `references/` (advanced mode only, #1242). */ + references: GeneratedSkillFile[]; + /** Text files to place under `assets/` (advanced mode only, #1242). */ + assets: GeneratedSkillFile[]; } export type SkillStreamEvent = From 9dea9af4db49d4a6615f1171215a7b82eaf1ff64 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:28:34 +0800 Subject: [PATCH 05/24] feat(api): mode-aware skill generation prompts (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the LLM-side half of the simple/advanced contract: - `SIMPLE_GENERATION_SYSTEM_PROMPT` — the schema block the model copies from lists only name / description / category "plain" / tags / readmeBody. Not offering `scripts`, `references`, `assets` or the runtime fields at all is the strongest lever before server validation; the prose additionally says why (the package is one SKILL.md) and how to handle API/tool tasks inline. - `GENERATION_SYSTEM_PROMPT` (advanced) now documents `references[]` (on-demand supporting docs) and `assets[]` (text-only run-time resources) with field rules and a worked example, so the model knows the package format allows them. - `getGenerationSystemPrompt(mode)` is the single selector; `buildDirectGenerationPrompt(query, mode = "advanced")` threads it so its previously unused `instructions` field becomes the source of truth for the service in the next commit. - `SIMPLE_MODE_RETRY_INSTRUCTION` — appended to the user turn on the one corrective retry a simple-mode violation gets. Tests pin the structural contract (no file arrays in the simple schema block, references/assets documented in the advanced prompt) rather than snapshotting prose. The hardcode sweep still passes — no URLs or model ids in the new text. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/skills/generation/prompts.test.ts | 68 ++++++++++- .../src/domains/skills/generation/prompts.ts | 115 ++++++++++++++++-- 2 files changed, 173 insertions(+), 10 deletions(-) diff --git a/ornn-api/src/domains/skills/generation/prompts.test.ts b/ornn-api/src/domains/skills/generation/prompts.test.ts index ec5cea90..623b0d73 100644 --- a/ornn-api/src/domains/skills/generation/prompts.test.ts +++ b/ornn-api/src/domains/skills/generation/prompts.test.ts @@ -1,7 +1,8 @@ /** * Unit tests for the skill-generation prompt builders (#875). * - * Three builders + three system-prompt constants are pinned here. The + * Three builders + four system-prompt constants (+ the mode selector, + * #1242) are pinned here. The * assertions are STRUCTURAL — they check that each conditional fragment * is present when (and only when) its option is supplied, plus the * fixed scaffolding the downstream parser / LLM relies on. We do NOT @@ -14,14 +15,17 @@ import { describe, expect, test } from "bun:test"; import { GENERATION_SYSTEM_PROMPT, OPENAPI_GENERATION_SYSTEM_PROMPT, + SIMPLE_GENERATION_SYSTEM_PROMPT, + SIMPLE_MODE_RETRY_INSTRUCTION, SOURCE_CODE_GENERATION_SYSTEM_PROMPT, buildDirectGenerationPrompt, buildOpenApiGenerationPrompt, buildSourceCodeGenerationPrompt, + getGenerationSystemPrompt, } from "./prompts"; describe("buildDirectGenerationPrompt", () => { - test("uses GENERATION_SYSTEM_PROMPT as instructions and embeds the query", () => { + test("defaults to the advanced GENERATION_SYSTEM_PROMPT and embeds the query", () => { const out = buildDirectGenerationPrompt("a web screenshot tool"); expect(out.instructions).toBe(GENERATION_SYSTEM_PROMPT); @@ -30,6 +34,18 @@ describe("buildDirectGenerationPrompt", () => { expect(out.userPrompt).toContain('Generate a skill for: "a web screenshot tool"'); }); + test("mode=simple swaps in SIMPLE_GENERATION_SYSTEM_PROMPT, same user scaffold (#1242)", () => { + const out = buildDirectGenerationPrompt("a web screenshot tool", "simple"); + expect(out.instructions).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + expect(out.userPrompt).toBe('Generate a skill for: "a web screenshot tool"'); + }); + + test("mode=advanced is the explicit spelling of the default", () => { + expect(buildDirectGenerationPrompt("x", "advanced").instructions).toBe( + GENERATION_SYSTEM_PROMPT, + ); + }); + test("preserves an empty query without leaking placeholder tokens", () => { const out = buildDirectGenerationPrompt(""); expect(out.userPrompt).toBe('Generate a skill for: ""'); @@ -139,9 +155,55 @@ describe("buildSourceCodeGenerationPrompt", () => { }); describe("system prompt constants", () => { - test("all three are non-empty", () => { + test("all four are non-empty", () => { expect(GENERATION_SYSTEM_PROMPT.length).toBeGreaterThan(0); + expect(SIMPLE_GENERATION_SYSTEM_PROMPT.length).toBeGreaterThan(0); expect(OPENAPI_GENERATION_SYSTEM_PROMPT.length).toBeGreaterThan(0); expect(SOURCE_CODE_GENERATION_SYSTEM_PROMPT.length).toBeGreaterThan(0); }); }); + +// ---- Mode-specific prompt contract (#1242) --------------------------- +// +// The simple prompt must not even OFFER the file arrays: the schema block +// the model copies from is the strongest lever we have before server-side +// validation kicks in. The advanced prompt must document every array the +// validator accepts so the model knows references/assets exist. + +describe("getGenerationSystemPrompt", () => { + test("simple → SIMPLE_GENERATION_SYSTEM_PROMPT, advanced → GENERATION_SYSTEM_PROMPT", () => { + expect(getGenerationSystemPrompt("simple")).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + expect(getGenerationSystemPrompt("advanced")).toBe(GENERATION_SYSTEM_PROMPT); + }); + + test("simple prompt offers no file arrays or runtime fields in its schema", () => { + // Fields the validator rejects in simple mode must be absent from + // the JSON SCHEMA block the model copies (they may still be named in + // the prose that forbids them, so scope the assertion to the block). + const schemaBlock = SIMPLE_GENERATION_SYSTEM_PROMPT.split("## JSON SCHEMA")[1]!.split("## EXAMPLE")[0]!; + for (const forbidden of ['"scripts"', '"references"', '"assets"', '"runtimes"', '"dependencies"', '"envVars"', '"outputType"']) { + expect(schemaBlock).not.toContain(forbidden); + } + expect(schemaBlock).toContain('"category": "plain"'); + expect(schemaBlock).toContain('"readmeBody"'); + }); + + test("simple prompt states the SKILL.md-only constraint in prose", () => { + expect(SIMPLE_GENERATION_SYSTEM_PROMPT).toContain("SKILL.md ONLY"); + expect(SIMPLE_GENERATION_SYSTEM_PROMPT).toContain('category is ALWAYS "plain"'); + }); + + test("advanced prompt documents references and assets alongside scripts", () => { + expect(GENERATION_SYSTEM_PROMPT).toContain('"references"'); + expect(GENERATION_SYSTEM_PROMPT).toContain('"assets"'); + expect(GENERATION_SYSTEM_PROMPT).toContain("**references**"); + expect(GENERATION_SYSTEM_PROMPT).toContain("**assets**"); + // Binary assets can't travel through the JSON contract. + expect(GENERATION_SYSTEM_PROMPT).toContain("TEXT ONLY"); + }); + + test("retry instruction names the simple-mode constraint and demands raw JSON", () => { + expect(SIMPLE_MODE_RETRY_INSTRUCTION).toContain("SIMPLE mode"); + expect(SIMPLE_MODE_RETRY_INSTRUCTION).toContain("Output ONLY valid JSON"); + }); +}); diff --git a/ornn-api/src/domains/skills/generation/prompts.ts b/ornn-api/src/domains/skills/generation/prompts.ts index a162d32d..98cf4d86 100644 --- a/ornn-api/src/domains/skills/generation/prompts.ts +++ b/ornn-api/src/domains/skills/generation/prompts.ts @@ -1,9 +1,25 @@ /** * Prompt templates for skill generation via Nyx Provider. - * Updated to include output-type field for runtime-based skills. + * + * The prompt-driven generator has two system prompts, one per + * {@link GenerationMode} (#1242): + * + * - `GENERATION_SYSTEM_PROMPT` (advanced) — the model may emit + * `scripts[]`, `references[]` and `assets[]` alongside the SKILL.md + * body. + * - `SIMPLE_GENERATION_SYSTEM_PROMPT` (simple) — the package is a + * single SKILL.md; the schema offered to the model does not even + * mention the file arrays so it has nothing to fill in. + * + * `getGenerationSystemPrompt(mode)` is the only selector the service + * should use. The OpenAPI / source-code prompts are intrinsically + * simple (plain, no files) and are unaffected by mode. + * * @module domains/skills/generation/prompts */ +import { DEFAULT_GENERATION_MODE, type GenerationMode } from "../../../shared/types/index"; + export const GENERATION_SYSTEM_PROMPT = `You are a skill generator for the ornn AI skill platform. Output ONLY a single JSON object. No markdown fences, no explanation, no extra text. ## CRITICAL: CHOOSING THE RIGHT CATEGORY @@ -52,7 +68,9 @@ Default to "node" for general web/API tasks. Use "python" for data science, ML, "runtimes": ["node"] or ["python"], "dependencies": ["package-name"], "envVars": ["ENV_VAR_NAME"], - "scripts": [{ "filename": "main.js", "content": "..." }] + "scripts": [{ "filename": "main.js", "content": "..." }], + "references": [{ "filename": "api-notes.md", "content": "..." }], + "assets": [{ "filename": "template.json", "content": "..." }] } ## EXAMPLE: PLAIN SKILL @@ -66,7 +84,9 @@ Default to "node" for general web/API tasks. Use "python" for data science, ML, "runtimes": [], "dependencies": [], "envVars": [], - "scripts": [] + "scripts": [], + "references": [], + "assets": [] } ## EXAMPLE: RUNTIME-BASED SKILL (Node.js) @@ -86,7 +106,9 @@ Default to "node" for general web/API tasks. Use "python" for data science, ML, "filename": "screenshot.js", "content": "const puppeteer = require('puppeteer');\\nconst url = process.env.TARGET_URL;\\nif (!url) { console.error('TARGET_URL required'); process.exit(1); }\\ntry {\\n const browser = await puppeteer.launch({ headless: true });\\n const page = await browser.newPage();\\n await page.goto(url, { waitUntil: 'networkidle2', timeout: 30000 });\\n await page.screenshot({ path: 'output.png', fullPage: true });\\n await browser.close();\\n console.log('Screenshot saved to output.png');\\n} catch (err) {\\n console.error('Failed:', err instanceof Error ? err.message : err);\\n process.exit(1);\\n}" } - ] + ], + "references": [], + "assets": [] } ## EXAMPLE: RUNTIME-BASED SKILL (Python) @@ -106,6 +128,18 @@ Default to "node" for general web/API tasks. Use "python" for data science, ML, "filename": "chart.py", "content": "import os\\nimport pandas as pd\\nimport matplotlib\\nmatplotlib.use('Agg')\\nimport matplotlib.pyplot as plt\\n\\nchart_type = os.environ.get('CHART_TYPE', 'bar')\\ndf = pd.read_csv('input.csv')\\n\\nfig, ax = plt.subplots(figsize=(10, 6))\\nif chart_type == 'pie':\\n ax.pie(df.iloc[:, 1], labels=df.iloc[:, 0], autopct='%1.1f%%')\\nelif chart_type == 'line':\\n ax.plot(df.iloc[:, 0], df.iloc[:, 1])\\nelse:\\n ax.bar(df.iloc[:, 0], df.iloc[:, 1])\\n\\nplt.tight_layout()\\nplt.savefig('chart.png', dpi=150)\\nprint('Chart saved to chart.png')" } + ], + "references": [ + { + "filename": "chart-types.md", + "content": "# Chart types\\n\\n| CHART_TYPE | Best for |\\n|---|---|\\n| bar | comparing categories |\\n| line | trends over time |\\n| pie | share of a whole (max ~6 slices) |" + } + ], + "assets": [ + { + "filename": "sample-input.csv", + "content": "label,value\\nQ1,120\\nQ2,180\\nQ3,150" + } ] } @@ -119,19 +153,86 @@ Default to "node" for general web/API tasks. Use "python" for data science, ML, - **runtimes**: ["node"] or ["python"] for runtime-based, [] for plain. Pick the best fit for the task. - **dependencies**: ONLY packages needed for scripts. [] for plain. NEVER include LLM SDKs (openai, anthropic, etc.). Use npm package names for node, pip package names for python. - **envVars**: ONLY for runtime-based scripts needing external config. [] for plain. +- **references**: OPTIONAL supporting documents the agent opens on demand — long API references, style guides, worked examples, decision tables. Keep readmeBody focused and move deep detail here. Markdown or plain text, .md/.txt extension. Allowed for any category. [] when not needed. +- **assets**: OPTIONAL text resources the skill uses at run time — templates, sample data, config snippets. TEXT ONLY (no binary, no images); .json/.csv/.txt/.yaml etc. Allowed for any category. [] when not needed. - **tags**: 1-10 lowercase kebab-case. Output ONLY the JSON object. Nothing else.`; /** - * Builds prompt for direct generation. + * System prompt for `simple` mode (#1242): the package is a single + * SKILL.md. The schema offered to the model deliberately omits every + * file array (scripts / references / assets) and every runtime field so + * there is nothing to fill in; the server still validates the answer. + */ +export const SIMPLE_GENERATION_SYSTEM_PROMPT = `You are a skill generator for the ornn AI skill platform. Output ONLY a single JSON object. No markdown fences, no explanation, no extra text. + +## SIMPLE MODE — SKILL.md ONLY + +The caller asked for a self-contained skill: the package is ONE SKILL.md file and nothing else. + +- category is ALWAYS "plain". +- Do NOT emit scripts, references, assets, runtimes, dependencies, envVars or outputType. The package cannot carry files. +- Everything the agent needs goes inline in readmeBody: purpose, step-by-step instructions, input/output format, worked examples, edge cases. +- If the task touches an external API or tool, explain exactly how to call it with the agent's own HTTP / shell abilities (endpoint, method, headers, body, example request and response). Inline command or code examples are fine AS DOCUMENTATION inside readmeBody — never as separate script files. +- If the request genuinely needs code execution, still produce a plain skill that tells the agent how to do it with the tools it already has. + +## JSON SCHEMA + +{ + "name": "kebab-case-name", + "description": "10-500 char description", + "category": "plain", + "tags": ["tag1", "tag2"], + "readmeBody": "markdown documentation body" +} + +## EXAMPLE + +{ + "name": "meeting-notes-to-action-items", + "description": "Turn raw meeting notes into a prioritised, owner-assigned action-item list.", + "category": "plain", + "tags": ["meetings", "summarisation", "productivity"], + "readmeBody": "# Meeting Notes to Action Items\\n\\n## Overview\\nExtract every commitment from free-form meeting notes and return them as an ordered action list.\\n\\n## Steps\\n1. Read the notes once end-to-end.\\n2. For each sentence that assigns work, capture: task, owner, due date (or \\"unspecified\\"), priority (P0-P2).\\n3. Merge duplicates; keep the most specific wording.\\n4. Sort by priority, then due date.\\n\\n## Output format\\n| # | Task | Owner | Due | Priority |\\n|---|------|-------|-----|----------|\\n\\n## Example\\nInput: \\"Sam will send the deck by Friday. We should also fix the login bug soon.\\"\\nOutput:\\n| 1 | Send the deck | Sam | Friday | P1 |\\n| 2 | Fix the login bug | unassigned | unspecified | P1 |\\n\\n## Edge cases\\n- No owner named → \\"unassigned\\".\\n- Vague timing (\\"soon\\") → \\"unspecified\\"; do not invent dates." +} + +## FIELD RULES + +- **name**: kebab-case ONLY. NO underscores. +- **description**: 10-500 chars. +- **category**: ALWAYS "plain". +- **readmeBody**: Markdown body. NO YAML frontmatter. Self-contained — the reader has nothing else. +- **tags**: 1-10 lowercase kebab-case. +- Do NOT include any other field. + +Output ONLY the JSON object. Nothing else.`; + +/** + * Appended to the user turn when a `simple`-mode answer carried files or + * a non-plain category and the service retries once (#1242). + */ +export const SIMPLE_MODE_RETRY_INSTRUCTION = + "IMPORTANT: Your previous answer included scripts, references, assets, runtime fields or a non-plain category. This is SIMPLE mode: output ONE plain skill with ONLY name, description, category \"plain\", tags and readmeBody. Fold anything that was in a script or reference file into readmeBody as documentation. Output ONLY valid JSON. No markdown fences. No extra text."; + +/** Select the prompt-driven system prompt for a generation mode. */ +export function getGenerationSystemPrompt(mode: GenerationMode): string { + return mode === "simple" ? SIMPLE_GENERATION_SYSTEM_PROMPT : GENERATION_SYSTEM_PROMPT; +} + +/** + * Builds prompt for direct generation. `instructions` is the mode's + * system prompt; the service sends it as a `developer` message. */ -export function buildDirectGenerationPrompt(query: string): { +export function buildDirectGenerationPrompt( + query: string, + mode: GenerationMode = DEFAULT_GENERATION_MODE, +): { instructions: string; userPrompt: string; } { return { - instructions: GENERATION_SYSTEM_PROMPT, + instructions: getGenerationSystemPrompt(mode), userPrompt: `Generate a skill for: "${query}"`, }; } From a9696ee110d28b21dc8dc6500a2b1d245342963d Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:32:35 +0800 Subject: [PATCH 06/24] feat(api): enforce generation mode in the generation service (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server-side half of the simple/advanced contract. `validation.ts` - `validateGeneratedSkill(raw, mode)` returns a discriminated result (`ok` | `invalid_json` | `schema` | `mode_violation`) so callers know *why* an answer was rejected and can pick the right retry instruction. In `simple` mode a schema-valid answer is a `mode_violation` when `category !== "plain"` or any of `scripts`, `references`, `assets`, `runtimes`, `dependencies`, `envVars` is non-empty; the message names the offending fields. - `parseGeneratedSkill` stays as the boolean-style wrapper the intrinsically-simple OpenAPI / source-code generators use. `service.ts` - `generateStream` / `generateStreamWithHistory` take a `GenerateOptions` object (`signal`, `modelOverride`, `mode`) instead of positional args; `mode` defaults to `advanced` and selects both the system prompt and the validator. - Single-turn: every first rejection still gets exactly one non-streaming retry, now with the instruction that matches the rejection (simple-mode nudge vs. "output valid JSON"). The retry is validated against the same mode, so "invalid JSON first, scripted on retry" still ends in `error`. - Multi-turn: invalid-JSON answers keep the existing no-retry rule (a refinement turn may legitimately be prose). A simple-mode violation is the one case that now retries — as a continuation of the conversation (offending answer → assistant turn, corrective instruction → user turn) — because the "SKILL.md only" guarantee is what agents integrate against. A second violation ends the stream with `error` and no `generation_complete`. - `parseAndValidate` is removed from the service; its tests move to `validation.test.ts`. Routes pass `{ signal, modelOverride }` for now; the `mode` field on the request body lands in the next commit. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/skills/generation/routes.test.ts | 6 +- .../src/domains/skills/generation/routes.ts | 5 +- .../domains/skills/generation/service.test.ts | 330 +++++++++++------- .../src/domains/skills/generation/service.ts | 214 +++++++++--- .../skills/generation/validation.test.ts | 223 ++++++++++++ .../domains/skills/generation/validation.ts | 137 ++++++-- 6 files changed, 704 insertions(+), 211 deletions(-) create mode 100644 ornn-api/src/domains/skills/generation/validation.test.ts diff --git a/ornn-api/src/domains/skills/generation/routes.test.ts b/ornn-api/src/domains/skills/generation/routes.test.ts index 2f97238c..f9f41f52 100644 --- a/ornn-api/src/domains/skills/generation/routes.test.ts +++ b/ornn-api/src/domains/skills/generation/routes.test.ts @@ -35,6 +35,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { Hono } from "hono"; import JSZip from "jszip"; import { createGenerationRoutes, type GenerationRoutesConfig } from "./routes"; +import type { GenerateOptions } from "./service"; import { __resetRateLimitForTests } from "../../../middleware/rateLimit"; import { buildProblemJsonBody } from "../../../shared/types/index"; import type { SkillStreamEvent } from "../../../shared/types/index"; @@ -97,10 +98,9 @@ class FakeGenerationService { generateStream( query: string, - _signal?: AbortSignal, - modelOverride?: string, + options: GenerateOptions = {}, ): AsyncIterable { - this.generateStreamCalls.push({ query, modelOverride }); + this.generateStreamCalls.push({ query, modelOverride: options.modelOverride }); return this.emit(); } diff --git a/ornn-api/src/domains/skills/generation/routes.ts b/ornn-api/src/domains/skills/generation/routes.ts index 508584c2..e9965801 100644 --- a/ornn-api/src/domains/skills/generation/routes.ts +++ b/ornn-api/src/domains/skills/generation/routes.ts @@ -136,8 +136,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V c, generationService.generateStreamWithHistory( body.messages as Array<{ role: "user" | "assistant"; content: string }>, - c.req.raw.signal, - pf.modelId, + { signal: c.req.raw.signal, modelOverride: pf.modelId }, ), keepAliveMs, { quotaService, userId: pf.userId, permissions: pf.permissions, modelId: pf.modelId, reservedAt: pf.reservedAt }, @@ -171,7 +170,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V const keepAliveMs = await resolveKeepAliveMs(keepAliveIntervalMsResolver); return streamGenerationEvents( c, - generationService.generateStream(query, signal, pf.modelId), + generationService.generateStream(query, { signal, modelOverride: pf.modelId }), keepAliveMs, { quotaService, userId: pf.userId, permissions: pf.permissions, modelId: pf.modelId, reservedAt: pf.reservedAt }, ); diff --git a/ornn-api/src/domains/skills/generation/service.test.ts b/ornn-api/src/domains/skills/generation/service.test.ts index f522b9e5..ea64997b 100644 --- a/ornn-api/src/domains/skills/generation/service.test.ts +++ b/ornn-api/src/domains/skills/generation/service.test.ts @@ -21,8 +21,10 @@ * passthrough / non-retry validation_error / pass / abort + throw. * - generateFromOpenApi / generateFromSource: happy + invalid + * option pass-through. - * - parseAndValidate: fence strip / brace slice / readmeMd migration - * (with + without frontmatter) / schema-fail / non-JSON. + * - mode (#1242): simple/advanced prompt selection, simple-mode + * violation → corrective retry → success / error on both the + * single-turn and multi-turn paths, advanced pass-through of + * references/assets. Parsing itself is pinned in validation.test.ts. * * @module domains/skills/generation/service.test */ @@ -41,6 +43,11 @@ import type { ResponsesApiOutput, } from "../../../clients/nyxid/llm"; import type { SkillStreamEvent } from "../../../shared/types/index"; +import { + GENERATION_SYSTEM_PROMPT, + SIMPLE_GENERATION_SYSTEM_PROMPT, + SIMPLE_MODE_RETRY_INSTRUCTION, +} from "./prompts"; // ---- Fixtures -------------------------------------------------------- @@ -64,6 +71,23 @@ const VALID_SKILL = JSON.stringify({ scripts: [], }); +/** Schema-valid but carries a script — legal in advanced, illegal in simple. */ +const SCRIPTED_SKILL = JSON.stringify({ + name: "scripted-skill", + description: "A runtime-based skill that ships a script and a reference.", + category: "runtime-based", + outputType: "text", + tags: ["demo"], + readmeBody: + "# Scripted Skill\n\nThis readme body is comfortably over the fifty character minimum length.", + runtimes: ["node"], + dependencies: ["axios"], + envVars: ["API_KEY"], + scripts: [{ filename: "main.js", content: "console.log('hi')" }], + references: [{ filename: "notes.md", content: "# Notes" }], + assets: [], +}); + // ---- Responses-API stream frame helpers ------------------------------ /** `response.output_text.delta` frame ({ delta: string }). */ @@ -215,7 +239,7 @@ describe("resolveDefaults", () => { llmClient: client, defaultsResolver: makeResolver(DEFAULTS), }); - await drain(svc.generateStream("q", undefined, "override-model")); + await drain(svc.generateStream("q", { modelOverride: "override-model" })); expect(streamParams[0]!.model).toBe("override-model"); }); @@ -324,7 +348,7 @@ describe("generateStream", () => { }); const ctrl = new AbortController(); ctrl.abort(); - const events = await drain(svc.generateStream("q", ctrl.signal)); + const events = await drain(svc.generateStream("q", { signal: ctrl.signal })); expect(types(events)).toEqual(["error"]); expect(streamParams).toHaveLength(0); }); @@ -341,7 +365,7 @@ describe("generateStream", () => { llmClient: client, defaultsResolver: makeResolver(DEFAULTS), }); - const events = await drain(svc.generateStream("q", ctrl.signal)); + const events = await drain(svc.generateStream("q", { signal: ctrl.signal })); expect(types(events)).toContain("error"); expect(types(events)).not.toContain("generation_complete"); }); @@ -415,6 +439,107 @@ describe("generateStream", () => { }); }); +// ---- generateStream × mode (#1242) ----------------------------------- + +describe("generateStream mode", () => { + function make(opts: FakeClientOpts) { + const made = makeClient(opts); + const svc = new SkillGenerationService({ + llmClient: made.client, + defaultsResolver: makeResolver(DEFAULTS), + }); + return { ...made, svc }; + } + + test("default mode is advanced: scripted output passes and the advanced prompt is sent", async () => { + const { svc, streamParams } = make({ streamFrames: [outputTextDelta(SCRIPTED_SKILL)] }); + const events = await drain(svc.generateStream("q")); + expect(types(events)).toEqual(["generation_start", "token", "generation_complete"]); + expect(streamParams[0]!.input[0]!.content).toBe(GENERATION_SYSTEM_PROMPT); + }); + + test("mode=advanced: references/assets travel through generation_complete.raw", async () => { + const { svc } = make({ streamFrames: [outputTextDelta(SCRIPTED_SKILL)] }); + const events = await drain(svc.generateStream("q", { mode: "advanced" })); + const complete = events.find((e) => e.type === "generation_complete") as { raw: string }; + expect(JSON.parse(complete.raw).references).toHaveLength(1); + }); + + test("mode=simple sends the simple prompt and accepts a plain SKILL.md-only answer", async () => { + const { svc, streamParams, completeParams } = make({ streamFrames: [outputTextDelta(VALID_SKILL)] }); + const events = await drain(svc.generateStream("q", { mode: "simple" })); + expect(types(events)).toEqual(["generation_start", "token", "generation_complete"]); + expect(streamParams[0]!.input[0]!.content).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + expect(completeParams).toHaveLength(0); + }); + + test("mode=simple: scripted answer → validation_error(retrying) → corrective retry → complete", async () => { + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(VALID_SKILL), + }); + const events = await drain(svc.generateStream("q", { mode: "simple" })); + expect(types(events)).toEqual([ + "generation_start", + "token", + "validation_error", + "generation_complete", + ]); + const ve = events.find((e) => e.type === "validation_error") as { message: string; retrying: boolean }; + expect(ve.retrying).toBe(true); + expect(ve.message).toContain("Simple mode"); + expect(ve.message).toContain("scripts"); + // The retry carries the simple-mode instruction, not the generic JSON one. + expect(completeParams).toHaveLength(1); + const retryUser = completeParams[0]!.input.at(-1)!.content; + expect(retryUser).toContain(SIMPLE_MODE_RETRY_INSTRUCTION); + expect(completeParams[0]!.input[0]!.content).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + const complete = events.find((e) => e.type === "generation_complete") as { raw: string }; + expect(complete.raw).toBe(VALID_SKILL); + }); + + test("mode=simple: retry that still carries files ends in error, never generation_complete", async () => { + const { svc } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(SCRIPTED_SKILL), + }); + const events = await drain(svc.generateStream("q", { mode: "simple" })); + expect(types(events)).not.toContain("generation_complete"); + const err = events.find((e) => e.type === "error") as { message: string }; + expect(err.message).toContain("simple mode after retry"); + }); + + test("mode=simple: invalid JSON first, scripted on retry → error (guarantee holds across reasons)", async () => { + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta("not json")], + completeResult: completeOutput(SCRIPTED_SKILL), + }); + const events = await drain(svc.generateStream("q", { mode: "simple" })); + // First rejection was JSON, so the generic instruction is used … + expect(completeParams[0]!.input.at(-1)!.content).not.toContain(SIMPLE_MODE_RETRY_INSTRUCTION); + // … but the retry is still validated against simple mode. + expect(types(events)).not.toContain("generation_complete"); + expect(types(events)).toContain("error"); + }); + + test("mode=simple: abort flipped after the first answer skips the retry", async () => { + const ctrl = new AbortController(); + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(VALID_SKILL), + // Abort AFTER the last frame is yielded: the stream loop sees the + // signal only on the next iteration, so accumulation completes and + // validation runs, but the retry must not fire. + onFrame: () => ctrl.abort(), + }); + const events = await drain(svc.generateStream("q", { mode: "simple", signal: ctrl.signal })); + expect(completeParams).toHaveLength(0); + expect(types(events)).toContain("validation_error"); + expect(types(events)).toContain("error"); + expect(types(events)).not.toContain("generation_complete"); + }); +}); + // ---- generateStreamWithHistory --------------------------------------- describe("generateStreamWithHistory", () => { @@ -488,7 +613,7 @@ describe("generateStreamWithHistory", () => { const ctrl = new AbortController(); ctrl.abort(); const events = await drain( - svc.generateStreamWithHistory([{ role: "user", content: "x" }], ctrl.signal), + svc.generateStreamWithHistory([{ role: "user", content: "x" }], { signal: ctrl.signal }), ); expect(types(events)).toEqual(["error"]); expect(streamParams).toHaveLength(0); @@ -512,6 +637,83 @@ describe("generateStreamWithHistory", () => { }); }); +// ---- generateStreamWithHistory × mode (#1242) ------------------------ + +describe("generateStreamWithHistory mode", () => { + function make(opts: FakeClientOpts) { + const made = makeClient(opts); + const svc = new SkillGenerationService({ + llmClient: made.client, + defaultsResolver: makeResolver(DEFAULTS), + }); + return { ...made, svc }; + } + const turn = [{ role: "user" as const, content: "x" }]; + + test("mode=simple selects the simple system prompt", async () => { + const { svc, streamParams } = make({ streamFrames: [outputTextDelta(VALID_SKILL)] }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + expect(streamParams[0]!.input[0]!.content).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + expect(types(events)).toEqual(["generation_start", "token", "generation_complete"]); + }); + + test("mode=advanced (default) accepts scripted output without retry", async () => { + const { svc, completeParams } = make({ streamFrames: [outputTextDelta(SCRIPTED_SKILL)] }); + const events = await drain(svc.generateStreamWithHistory(turn)); + expect(types(events)).toEqual(["generation_start", "token", "generation_complete"]); + expect(completeParams).toHaveLength(0); + }); + + test("mode=simple: invalid JSON still follows the no-retry multi-turn rule", async () => { + const { svc, completeParams } = make({ streamFrames: [outputTextDelta("prose reply")] }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + expect(completeParams).toHaveLength(0); + const ve = events.find((e) => e.type === "validation_error") as { retrying: boolean }; + expect(ve.retrying).toBe(false); + expect(types(events)).toContain("generation_complete"); + }); + + test("mode=simple: scripted answer is retried as a conversation turn and can succeed", async () => { + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(VALID_SKILL), + }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + expect(types(events)).toEqual([ + "generation_start", + "token", + "validation_error", + "generation_complete", + ]); + expect((events[2] as { retrying: boolean }).retrying).toBe(true); + // Retry input = original conversation + offending assistant turn + corrective user turn. + const input = completeParams[0]!.input; + expect(input.at(-2)).toEqual({ role: "assistant", content: SCRIPTED_SKILL }); + expect(input.at(-1)).toEqual({ role: "user", content: SIMPLE_MODE_RETRY_INSTRUCTION }); + expect((events[3] as { raw: string }).raw).toBe(VALID_SKILL); + }); + + test("mode=simple: scripted answer twice ends in error with no generation_complete", async () => { + const { svc } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(SCRIPTED_SKILL), + }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + expect(types(events)).toEqual(["generation_start", "token", "validation_error", "error"]); + }); + + test("mode=simple: retry call throwing surfaces an LLM retry error", async () => { + const { svc } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeThrow: new Error("retry 502"), + }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + const err = events.find((e) => e.type === "error") as { message: string }; + expect(err.message).toContain("retry 502"); + expect(types(events)).not.toContain("generation_complete"); + }); +}); + // ---- generateFromOpenApi --------------------------------------------- describe("generateFromOpenApi", () => { @@ -591,119 +793,3 @@ describe("generateFromSource", () => { expect(types(events)).toContain("generation_complete"); }); }); - -// ---- parseAndValidate (direct) --------------------------------------- - -describe("parseAndValidate", () => { - function svc(): SkillGenerationService { - const { client } = makeClient({}); - return new SkillGenerationService({ - llmClient: client, - defaultsResolver: makeResolver(DEFAULTS), - }); - } - - test("strips a ```json fence", () => { - const out = svc().parseAndValidate("```json\n" + VALID_SKILL + "\n```"); - expect(out).not.toBeNull(); - expect(out!.name).toBe("demo-skill"); - }); - - test("strips a bare ``` fence", () => { - const out = svc().parseAndValidate("```\n" + VALID_SKILL + "\n```"); - expect(out).not.toBeNull(); - }); - - test("slices the brace span out of prose-wrapped output", () => { - const out = svc().parseAndValidate( - "Sure! Here is your skill:\n" + VALID_SKILL + "\nHope that helps.", - ); - expect(out).not.toBeNull(); - expect(out!.name).toBe("demo-skill"); - }); - - test("migrates readmeMd → readmeBody, stripping YAML frontmatter", () => { - const withFrontmatter = JSON.stringify({ - name: "legacy-skill", - description: "A legacy skill carrying readmeMd with frontmatter.", - category: "plain", - tags: ["legacy"], - readmeMd: - "---\ntitle: Legacy\nfoo: bar\n---\n# Legacy Skill\n\nBody content that is well over the fifty character minimum requirement.", - runtimes: [], - dependencies: [], - envVars: [], - scripts: [], - }); - const out = svc().parseAndValidate(withFrontmatter); - expect(out).not.toBeNull(); - expect(out!.readmeBody).toContain("# Legacy Skill"); - expect(out!.readmeBody).not.toContain("title: Legacy"); - }); - - test("migrates readmeMd → readmeBody when there is no frontmatter", () => { - const noFrontmatter = JSON.stringify({ - name: "legacy-plain", - description: "A legacy skill carrying readmeMd without frontmatter.", - category: "plain", - tags: ["legacy"], - readmeMd: - "# Plain Legacy\n\nThis body has no YAML frontmatter and is over the fifty char minimum.", - runtimes: [], - dependencies: [], - envVars: [], - scripts: [], - }); - const out = svc().parseAndValidate(noFrontmatter); - expect(out).not.toBeNull(); - expect(out!.readmeBody).toContain("# Plain Legacy"); - }); - - test("schema violation returns null", () => { - const badSchema = JSON.stringify({ - name: "Bad Name With Spaces", - description: "short", - category: "plain", - tags: [], - readmeBody: "too short", - runtimes: [], - dependencies: [], - envVars: [], - scripts: [], - }); - expect(svc().parseAndValidate(badSchema)).toBeNull(); - }); - - test("non-JSON input returns null", () => { - expect(svc().parseAndValidate("this is not json at all")).toBeNull(); - }); - - test("references / assets default to [] when the model omits them (#1242)", () => { - const out = svc().parseAndValidate(VALID_SKILL); - expect(out).not.toBeNull(); - expect(out!.references).toEqual([]); - expect(out!.assets).toEqual([]); - }); - - test("references / assets pass through when the model emits them (#1242)", () => { - const withExtras = JSON.stringify({ - ...JSON.parse(VALID_SKILL), - references: [{ filename: "api.md", content: "# API\n\nReference material." }], - assets: [{ filename: "template.json", content: "{\"greeting\":\"hi\"}" }], - }); - const out = svc().parseAndValidate(withExtras); - expect(out).not.toBeNull(); - expect(out!.references).toEqual([ - { filename: "api.md", content: "# API\n\nReference material." }, - ]); - expect(out!.assets[0]!.filename).toBe("template.json"); - }); - - test("a references entry with empty content fails the schema (#1242)", () => { - const bad = JSON.stringify({ - ...JSON.parse(VALID_SKILL), - references: [{ filename: "empty.md", content: "" }], - }); - expect(svc().parseAndValidate(bad)).toBeNull(); - }); -}); diff --git a/ornn-api/src/domains/skills/generation/service.ts b/ornn-api/src/domains/skills/generation/service.ts index 866a3bfe..318a45bc 100644 --- a/ornn-api/src/domains/skills/generation/service.ts +++ b/ornn-api/src/domains/skills/generation/service.ts @@ -6,19 +6,32 @@ */ import type { NyxLlmClient, ResponsesApiStreamEvent, ResponsesApiInputMessage } from "../../../clients/nyxid/llm"; -import type { GeneratedSkill, SkillStreamEvent } from "../../../shared/types/index"; +import { + DEFAULT_GENERATION_MODE, + type GenerationMode, + type SkillStreamEvent, +} from "../../../shared/types/index"; import { buildDirectGenerationPrompt, buildOpenApiGenerationPrompt, buildSourceCodeGenerationPrompt, - GENERATION_SYSTEM_PROMPT, + getGenerationSystemPrompt, OPENAPI_GENERATION_SYSTEM_PROMPT, + SIMPLE_MODE_RETRY_INSTRUCTION, SOURCE_CODE_GENERATION_SYSTEM_PROMPT, } from "./prompts"; -import { parseGeneratedSkill } from "./validation"; +import { + parseGeneratedSkill, + validateGeneratedSkill, + type GeneratedSkillValidation, +} from "./validation"; import { createLogger } from "../../../shared/logger"; const logger = createLogger("skillGenerationService"); +/** Appended to the user turn when the first answer was not valid JSON. */ +const JSON_RETRY_INSTRUCTION = + "IMPORTANT: Output ONLY valid JSON. No markdown fences. No extra text."; + /** * Per-call resolution of LLM defaults from admin settings (`skillGen` * section + selected provider's `maxOutputTokens` / `defaultTemperature`). @@ -49,6 +62,18 @@ interface LlmCallContext { defaults: SkillGenLlmDefaults; } +/** + * Per-call options for the prompt-driven generators (#1242). Optional + * members widen with `| undefined` for exactOptionalPropertyTypes (#657). + */ +export interface GenerateOptions { + signal?: AbortSignal | undefined; + /** Admin-curated model id; the surface default applies when unset. */ + modelOverride?: string | undefined; + /** Package shape the caller asked for. Defaults to `advanced`. */ + mode?: GenerationMode | undefined; +} + export class SkillGenerationService { private readonly llmClient: NyxLlmClient; private readonly defaultsResolver: SkillGenLlmDefaultsResolver; @@ -162,80 +187,138 @@ export class SkillGenerationService { return text; } + /** + * Pick the retry instruction that addresses the actual rejection: a + * simple-mode violation gets the "fold everything into SKILL.md" + * nudge, anything else the plain "output valid JSON" one. + */ + private static retryInstructionFor(rejection: GeneratedSkillValidation): string { + return !rejection.ok && rejection.reason === "mode_violation" + ? SIMPLE_MODE_RETRY_INSTRUCTION + : JSON_RETRY_INSTRUCTION; + } + + /** Terminal `error` message once the retry also failed. */ + private static exhaustedMessageFor(rejection: GeneratedSkillValidation): string { + return !rejection.ok && rejection.reason === "mode_violation" + ? "LLM produced advanced materials in simple mode after retry" + : "LLM produced invalid output after retry"; + } + + /** + * One non-streaming retry after a rejected answer. Yields the terminal + * frame — `generation_complete` on success, `error` otherwise — so the + * caller just returns afterwards. + */ + private async *retryOnce( + retryInput: ResponsesApiInputMessage[], + ctx: LlmCallContext, + mode: GenerationMode, + firstRejection: GeneratedSkillValidation, + ): AsyncGenerator { + let retryText: string; + try { + retryText = await this.completeLlm(retryInput, ctx); + } catch (retryErr) { + const msg = retryErr instanceof Error ? retryErr.message : String(retryErr); + logger.error({ err: msg, mode }, "LLM retry error"); + yield { type: "error", message: `LLM retry error: ${msg}` }; + return; + } + + const retried = validateGeneratedSkill(retryText, mode); + if (retried.ok) { + logger.info({ mode, skillName: retried.skill.name }, "Generation retry passed validation"); + yield { type: "generation_complete", raw: retryText }; + return; + } + + logger.warn( + { mode, firstReason: firstRejection.ok ? null : firstRejection.reason, retryReason: retried.reason, violations: retried.violations }, + "Generation retry failed validation", + ); + yield { type: "error", message: SkillGenerationService.exhaustedMessageFor(retried) }; + } + /** * Direct generation streaming. Streams tokens via SSE events. * Uses Nyx Provider Responses API format. `modelOverride` (when set) * picks an admin-curated model; otherwise the service-level default - * applies. + * applies. `mode` selects the system prompt and the package-shape + * validation (#1242). + * + * Any rejected first answer (invalid JSON, schema failure, or a + * simple-mode violation) is retried once with a corrective + * instruction; a second rejection ends the stream with `error` and no + * `generation_complete`. */ async *generateStream( query: string, - signal?: AbortSignal, - modelOverride?: string, + options: GenerateOptions = {}, ): AsyncIterable { + const { signal, modelOverride } = options; + const mode = options.mode ?? DEFAULT_GENERATION_MODE; const ctx = yield* this.begin(signal, modelOverride); if (!ctx) return; - const { userPrompt } = buildDirectGenerationPrompt(query); + const { instructions, userPrompt } = buildDirectGenerationPrompt(query, mode); const input: ResponsesApiInputMessage[] = [ - { role: "developer", content: GENERATION_SYSTEM_PROMPT }, + { role: "developer", content: instructions }, { role: "user", content: userPrompt }, ]; const accumulated = yield* this.streamLlm(input, ctx, signal, "direct"); if (accumulated === null) return; - // Validate the accumulated output - const parsed = this.parseAndValidate(accumulated); - if (!parsed) { - logger.warn("LLM output failed validation, attempting retry"); - yield { type: "validation_error", message: "Invalid JSON from LLM", retrying: true }; - - // Retry with non-streaming complete call - if (!signal?.aborted) { - try { - const retryInput: ResponsesApiInputMessage[] = [ - { role: "developer", content: GENERATION_SYSTEM_PROMPT }, - { role: "user", content: `${userPrompt}\n\nIMPORTANT: Output ONLY valid JSON. No markdown fences. No extra text.` }, - ]; - - const retryText = await this.completeLlm(retryInput, ctx); - - const retryParsed = this.parseAndValidate(retryText); - if (retryParsed) { - yield { type: "generation_complete", raw: retryText }; - return; - } - } catch (retryErr) { - const msg = retryErr instanceof Error ? retryErr.message : String(retryErr); - logger.error({ err: msg }, "LLM retry error"); - yield { type: "error", message: `LLM retry error: ${msg}` }; - return; - } - } + const validation = validateGeneratedSkill(accumulated, mode); + if (validation.ok) { + yield { type: "generation_complete", raw: accumulated }; + return; + } + + logger.warn( + { mode, reason: validation.reason, violations: validation.violations }, + "LLM output failed validation, attempting retry", + ); + yield { type: "validation_error", message: validation.message, retrying: true }; - yield { type: "error", message: "LLM produced invalid output after retry" }; + if (signal?.aborted) { + yield { type: "error", message: SkillGenerationService.exhaustedMessageFor(validation) }; return; } - yield { type: "generation_complete", raw: accumulated }; + const retryInput: ResponsesApiInputMessage[] = [ + { role: "developer", content: instructions }, + { role: "user", content: `${userPrompt}\n\n${SkillGenerationService.retryInstructionFor(validation)}` }, + ]; + yield* this.retryOnce(retryInput, ctx, mode, validation); } /** * Multi-turn generation. Converts message history to Responses API format. + * + * Unlike the single-turn path this does NOT retry an answer that is + * merely not valid JSON — a refinement turn may legitimately be prose + * (the model asking a question), so the raw text is still delivered in + * `generation_complete` after a `validation_error` with + * `retrying: false`. The one exception is a simple-mode violation + * (#1242): the "SKILL.md only" guarantee is load-bearing for agents, + * so that case gets one corrective retry and ends in `error` if the + * model still emits files. */ async *generateStreamWithHistory( messages: Array<{ role: "user" | "assistant"; content: string }>, - signal?: AbortSignal, - modelOverride?: string, + options: GenerateOptions = {}, ): AsyncIterable { + const { signal, modelOverride } = options; + const mode = options.mode ?? DEFAULT_GENERATION_MODE; const ctx = yield* this.begin(signal, modelOverride); if (!ctx) return; // Put system prompt as developer message in input array (not as instructions) // because some LLM providers ignore the instructions field. const input: ResponsesApiInputMessage[] = [ - { role: "developer", content: GENERATION_SYSTEM_PROMPT }, + { role: "developer", content: getGenerationSystemPrompt(mode) }, ...messages.map((m, i) => { if (i === 0 && m.role === "user") { return { @@ -254,19 +337,45 @@ export class SkillGenerationService { if (accumulated === null) return; logger.info( - { accumulatedLength: accumulated.length, first200: accumulated.slice(0, 200), last200: accumulated.slice(-200) }, + { mode, accumulatedLength: accumulated.length, first200: accumulated.slice(0, 200), last200: accumulated.slice(-200) }, "Multi-turn generation accumulated text", ); - const parsed = this.parseAndValidate(accumulated); - if (!parsed) { - logger.warn({ first500: accumulated.slice(0, 500) }, "Multi-turn validation failed"); - yield { type: "validation_error", message: "Invalid JSON from LLM", retrying: false }; - } else { - logger.info({ skillName: parsed.name }, "Multi-turn validation passed"); + const validation = validateGeneratedSkill(accumulated, mode); + if (validation.ok) { + logger.info({ mode, skillName: validation.skill.name }, "Multi-turn validation passed"); + yield { type: "generation_complete", raw: accumulated }; + return; } - yield { type: "generation_complete", raw: accumulated }; + if (validation.reason !== "mode_violation") { + logger.warn({ mode, reason: validation.reason, first500: accumulated.slice(0, 500) }, "Multi-turn validation failed"); + yield { type: "validation_error", message: validation.message, retrying: false }; + yield { type: "generation_complete", raw: accumulated }; + return; + } + + logger.warn( + { mode, violations: validation.violations }, + "Multi-turn output violates simple mode, attempting retry", + ); + yield { type: "validation_error", message: validation.message, retrying: true }; + + if (signal?.aborted) { + yield { type: "error", message: SkillGenerationService.exhaustedMessageFor(validation) }; + return; + } + + // Continue the conversation: the offending answer becomes an + // assistant turn and the corrective instruction the next user turn, + // so the model rewrites what it just produced rather than starting + // from the original prompt alone. + const retryInput: ResponsesApiInputMessage[] = [ + ...input, + { role: "assistant", content: accumulated }, + { role: "user", content: SIMPLE_MODE_RETRY_INSTRUCTION }, + ]; + yield* this.retryOnce(retryInput, ctx, mode, validation); } /** @@ -291,7 +400,7 @@ export class SkillGenerationService { const accumulated = yield* this.streamLlm(input, ctx, signal, "OpenAPI generation"); if (accumulated === null) return; - const parsed = this.parseAndValidate(accumulated); + const parsed = parseGeneratedSkill(accumulated); if (!parsed) { logger.warn("OpenAPI generation output failed validation"); yield { type: "validation_error", message: "Invalid JSON from LLM", retrying: false }; @@ -332,7 +441,7 @@ export class SkillGenerationService { const accumulated = yield* this.streamLlm(input, ctx, signal, "Source-code generation"); if (accumulated === null) return; - const parsed = this.parseAndValidate(accumulated); + const parsed = parseGeneratedSkill(accumulated); if (!parsed) { logger.warn("Source-code generation output failed validation"); yield { type: "validation_error", message: "Invalid JSON from LLM", retrying: false }; @@ -341,9 +450,6 @@ export class SkillGenerationService { yield { type: "generation_complete", raw: accumulated }; } - parseAndValidate(raw: string): GeneratedSkill | null { - return parseGeneratedSkill(raw); - } } /** diff --git a/ornn-api/src/domains/skills/generation/validation.test.ts b/ornn-api/src/domains/skills/generation/validation.test.ts new file mode 100644 index 00000000..5cd57405 --- /dev/null +++ b/ornn-api/src/domains/skills/generation/validation.test.ts @@ -0,0 +1,223 @@ +/** + * Unit tests for the generated-skill validator (#1242). + * + * The parse/clean cases (fence strip / brace slice / readmeMd migration + * / schema-fail / non-JSON) moved here from `service.test.ts` when the + * routine left the service. The mode cases pin the package-shape rule: + * `advanced` accepts every schema-valid answer, `simple` rejects any + * answer that carries files or runtime fields with a `mode_violation` + * that names the offending fields. + * + * @module domains/skills/generation/validation.test + */ + +import { describe, expect, test } from "bun:test"; +import { + findSimpleModeViolations, + parseGeneratedSkill, + validateGeneratedSkill, +} from "./validation"; + +/** A schema-valid, SKILL.md-only skill document. */ +const PLAIN = { + name: "demo-skill", + description: "A perfectly valid demo skill for testing purposes.", + category: "plain", + tags: ["demo", "test"], + readmeBody: + "# Demo Skill\n\nThis readme body is comfortably over the fifty character minimum length.", + runtimes: [], + dependencies: [], + envVars: [], + scripts: [], +}; +const PLAIN_JSON = JSON.stringify(PLAIN); + +/** Schema-valid, carries every advanced-mode field. */ +const SCRIPTED = { + ...PLAIN, + name: "scripted-skill", + category: "runtime-based", + outputType: "text", + runtimes: ["node"], + dependencies: ["axios"], + envVars: ["API_KEY"], + scripts: [{ filename: "main.js", content: "console.log('hi')" }], + references: [{ filename: "notes.md", content: "# Notes" }], + assets: [{ filename: "sample.csv", content: "a,b\n1,2" }], +}; +const SCRIPTED_JSON = JSON.stringify(SCRIPTED); + +// ---- parseGeneratedSkill -------------------------------------------- + +describe("parseGeneratedSkill", () => { + test("strips a ```json fence", () => { + const out = parseGeneratedSkill("```json\n" + PLAIN_JSON + "\n```"); + expect(out).not.toBeNull(); + expect(out!.name).toBe("demo-skill"); + }); + + test("strips a bare ``` fence", () => { + expect(parseGeneratedSkill("```\n" + PLAIN_JSON + "\n```")).not.toBeNull(); + }); + + test("slices the brace span out of prose-wrapped output", () => { + const out = parseGeneratedSkill( + "Sure! Here is your skill:\n" + PLAIN_JSON + "\nHope that helps.", + ); + expect(out).not.toBeNull(); + expect(out!.name).toBe("demo-skill"); + }); + + test("migrates readmeMd → readmeBody, stripping YAML frontmatter", () => { + const withFrontmatter = JSON.stringify({ + name: "legacy-skill", + description: "A legacy skill carrying readmeMd with frontmatter.", + category: "plain", + tags: ["legacy"], + readmeMd: + "---\ntitle: Legacy\nfoo: bar\n---\n# Legacy Skill\n\nBody content that is well over the fifty character minimum requirement.", + }); + const out = parseGeneratedSkill(withFrontmatter); + expect(out).not.toBeNull(); + expect(out!.readmeBody).toContain("# Legacy Skill"); + expect(out!.readmeBody).not.toContain("title: Legacy"); + }); + + test("migrates readmeMd → readmeBody when there is no frontmatter", () => { + const noFrontmatter = JSON.stringify({ + name: "legacy-plain", + description: "A legacy skill carrying readmeMd without frontmatter.", + category: "plain", + tags: ["legacy"], + readmeMd: + "# Plain Legacy\n\nThis body has no YAML frontmatter and is over the fifty char minimum.", + }); + const out = parseGeneratedSkill(noFrontmatter); + expect(out).not.toBeNull(); + expect(out!.readmeBody).toContain("# Plain Legacy"); + }); + + test("schema violation returns null", () => { + const badSchema = JSON.stringify({ + name: "Bad Name With Spaces", + description: "short", + category: "plain", + tags: [], + readmeBody: "too short", + }); + expect(parseGeneratedSkill(badSchema)).toBeNull(); + }); + + test("non-JSON input returns null", () => { + expect(parseGeneratedSkill("this is not json at all")).toBeNull(); + }); + + test("references / assets default to [] when the model omits them", () => { + const out = parseGeneratedSkill(PLAIN_JSON); + expect(out!.references).toEqual([]); + expect(out!.assets).toEqual([]); + }); + + test("references / assets pass through when the model emits them", () => { + const out = parseGeneratedSkill(SCRIPTED_JSON); + expect(out!.references).toEqual([{ filename: "notes.md", content: "# Notes" }]); + expect(out!.assets[0]!.filename).toBe("sample.csv"); + }); + + test("a references entry with empty content fails the schema", () => { + const bad = JSON.stringify({ + ...PLAIN, + references: [{ filename: "empty.md", content: "" }], + }); + expect(parseGeneratedSkill(bad)).toBeNull(); + }); +}); + +// ---- findSimpleModeViolations ---------------------------------------- + +describe("findSimpleModeViolations", () => { + test("a plain SKILL.md-only skill has no violations", () => { + expect(findSimpleModeViolations(parseGeneratedSkill(PLAIN_JSON)!)).toEqual([]); + }); + + test("names every offending field, category first", () => { + expect(findSimpleModeViolations(parseGeneratedSkill(SCRIPTED_JSON)!)).toEqual([ + "category", + "scripts", + "references", + "assets", + "runtimes", + "dependencies", + "envVars", + ]); + }); + + test("a plain skill with only references is still a violation", () => { + const skill = parseGeneratedSkill( + JSON.stringify({ ...PLAIN, references: [{ filename: "r.md", content: "ref" }] }), + )!; + expect(findSimpleModeViolations(skill)).toEqual(["references"]); + }); + + test("a stray outputType on a plain skill is tolerated", () => { + const skill = parseGeneratedSkill(JSON.stringify({ ...PLAIN, outputType: "text" }))!; + expect(findSimpleModeViolations(skill)).toEqual([]); + }); +}); + +// ---- validateGeneratedSkill ------------------------------------------ + +describe("validateGeneratedSkill", () => { + test("advanced accepts a scripted answer", () => { + const r = validateGeneratedSkill(SCRIPTED_JSON, "advanced"); + expect(r.ok).toBe(true); + if (r.ok) expect(r.skill.name).toBe("scripted-skill"); + }); + + test("advanced accepts a plain answer", () => { + expect(validateGeneratedSkill(PLAIN_JSON, "advanced").ok).toBe(true); + }); + + test("simple accepts a plain SKILL.md-only answer", () => { + expect(validateGeneratedSkill(PLAIN_JSON, "simple").ok).toBe(true); + }); + + test("simple rejects a scripted answer as mode_violation naming the fields", () => { + const r = validateGeneratedSkill(SCRIPTED_JSON, "simple"); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.reason).toBe("mode_violation"); + expect(r.violations).toContain("scripts"); + expect(r.violations).toContain("references"); + expect(r.message).toContain("Simple mode allows SKILL.md only"); + expect(r.message).toContain("scripts"); + } + }); + + test("simple rejects a plain answer that sneaks in an asset", () => { + const r = validateGeneratedSkill( + JSON.stringify({ ...PLAIN, assets: [{ filename: "t.json", content: "{}" }] }), + "simple", + ); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.violations).toEqual(["assets"]); + }); + + test("invalid JSON is reported as invalid_json in either mode", () => { + for (const mode of ["simple", "advanced"] as const) { + const r = validateGeneratedSkill("nope", mode); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.reason).toBe("invalid_json"); + expect(r.violations).toEqual([]); + } + } + }); + + test("a schema failure is reported as schema, not mode_violation", () => { + const r = validateGeneratedSkill(JSON.stringify({ ...SCRIPTED, name: "Bad Name" }), "simple"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("schema"); + }); +}); diff --git a/ornn-api/src/domains/skills/generation/validation.ts b/ornn-api/src/domains/skills/generation/validation.ts index 60e39777..95679090 100644 --- a/ornn-api/src/domains/skills/generation/validation.ts +++ b/ornn-api/src/domains/skills/generation/validation.ts @@ -1,13 +1,17 @@ /** * Parsing + schema validation of the JSON document the LLM returns for - * a generated skill. Split out of `service.ts` (#1242) so the schema has - * one home and the service only orchestrates streaming. + * a generated skill, plus the per-mode package-shape check (#1242). + * + * Split out of `service.ts` so the schema has one home and the service + * only orchestrates streaming. `validateGeneratedSkill` is the single + * entry point; it tells the caller *why* an answer was rejected so the + * retry prompt can address the actual problem. * * @module domains/skills/generation/validation */ import { z } from "zod"; -import type { GeneratedSkill } from "../../../shared/types/index"; +import type { GeneratedSkill, GenerationMode } from "../../../shared/types/index"; import { createLogger } from "../../../shared/logger"; const logger = createLogger("skillGenerationValidation"); @@ -39,13 +43,63 @@ export const generatedSkillSchema = z.object({ assets: z.array(generatedFileSchema).default([]), }); +/** + * Why an answer was rejected. `mode_violation` is only ever produced in + * `simple` mode and is the one case the multi-turn path retries. + */ +export type GeneratedSkillRejection = "invalid_json" | "schema" | "mode_violation"; + +export type GeneratedSkillValidation = + | { ok: true; skill: GeneratedSkill } + | { + ok: false; + reason: GeneratedSkillRejection; + /** Human-readable summary, safe to send in a `validation_error` frame. */ + message: string; + /** Offending field names — set for `mode_violation` only. */ + violations: string[]; + }; + +/** + * Array fields that must be empty in `simple` mode. `category` is checked + * separately (must be `plain`). `outputType` is deliberately not listed: + * a stray `outputType` on a plain skill is harmless and the frontmatter + * builder ignores it. + */ +const SIMPLE_MODE_EMPTY_FIELDS = [ + "scripts", + "references", + "assets", + "runtimes", + "dependencies", + "envVars", +] as const; + +/** + * Names of the fields that make a schema-valid skill unacceptable in + * `simple` mode. Empty array ⇒ the skill is a legal simple package. + */ +export function findSimpleModeViolations(skill: GeneratedSkill): string[] { + const violations: string[] = []; + if (skill.category !== "plain") violations.push("category"); + for (const field of SIMPLE_MODE_EMPTY_FIELDS) { + if (skill[field].length > 0) violations.push(field); + } + return violations; +} + /** * Strip markdown fences / surrounding prose, parse the JSON object and * validate it against {@link generatedSkillSchema}. Returns `null` when - * the text is not a schema-valid skill document; callers decide whether - * to retry or give up. + * the text is not a schema-valid skill document. */ export function parseGeneratedSkill(raw: string): GeneratedSkill | null { + const result = parseGeneratedSkillDetailed(raw); + return result.ok ? result.skill : null; +} + +function parseGeneratedSkillDetailed(raw: string): GeneratedSkillValidation { + let json: Record; try { let cleaned = raw.replace(/```json\n?/g, "").replace(/```\n?/g, "").trim(); @@ -55,34 +109,59 @@ export function parseGeneratedSkill(raw: string): GeneratedSkill | null { cleaned = cleaned.slice(jsonStart, jsonEnd + 1); } - const json = JSON.parse(cleaned); - - // Handle backward-compat: rename readmeMd -> readmeBody - if (json.readmeMd && !json.readmeBody) { - const md = json.readmeMd as string; - const fmEnd = md.indexOf("\n---", 3); - json.readmeBody = fmEnd > 0 ? md.slice(fmEnd + 4).trim() : md; - delete json.readmeMd; - } - - const result = generatedSkillSchema.safeParse(json); - if (!result.success) { - logger.debug({ errors: result.error.issues }, "Generated skill validation failed"); - return null; - } - - // The Zod-inferred shape and GeneratedSkill match in spirit but - // Zod surfaces `outputType` as `"text" | "file" | undefined` - // (explicit undefined, not optional) which exactOptionalPropertyTypes - // (#657) treats as different from the interface's `outputType?:`. - // Same runtime shape; cast is safe. - return result.data as GeneratedSkill; + json = JSON.parse(cleaned); } catch (err) { - // Generated-skill JSON parse failed. Caller treats null as + // Generated-skill JSON parse failed. Caller treats this as // "regenerate" or "give up" depending on retry budget. Logging // so we can spot a model that's consistently producing // unparseable output (#579). logger.debug({ err }, "generated skill JSON parse failed"); - return null; + return { ok: false, reason: "invalid_json", message: "Invalid JSON from LLM", violations: [] }; + } + + // Handle backward-compat: rename readmeMd -> readmeBody + if (json.readmeMd && !json.readmeBody) { + const md = json.readmeMd as string; + const fmEnd = md.indexOf("\n---", 3); + json.readmeBody = fmEnd > 0 ? md.slice(fmEnd + 4).trim() : md; + delete json.readmeMd; + } + + const result = generatedSkillSchema.safeParse(json); + if (!result.success) { + logger.debug({ errors: result.error.issues }, "Generated skill validation failed"); + return { ok: false, reason: "schema", message: "Invalid JSON from LLM", violations: [] }; } + + // The Zod-inferred shape and GeneratedSkill match in spirit but + // Zod surfaces `outputType` as `"text" | "file" | undefined` + // (explicit undefined, not optional) which exactOptionalPropertyTypes + // (#657) treats as different from the interface's `outputType?:`. + // Same runtime shape; cast is safe. + return { ok: true, skill: result.data as GeneratedSkill }; +} + +/** + * Parse + schema-validate, then apply the package-shape rule for `mode`. + * In `advanced` mode every schema-valid answer is accepted; in `simple` + * mode an answer that carries scripts / references / assets / runtime + * fields or a non-plain category is rejected as a `mode_violation`. + */ +export function validateGeneratedSkill( + raw: string, + mode: GenerationMode, +): GeneratedSkillValidation { + const parsed = parseGeneratedSkillDetailed(raw); + if (!parsed.ok || mode !== "simple") return parsed; + + const violations = findSimpleModeViolations(parsed.skill); + if (violations.length === 0) return parsed; + + logger.debug({ violations }, "Generated skill violates simple mode"); + return { + ok: false, + reason: "mode_violation", + message: `Simple mode allows SKILL.md only, but the model emitted: ${violations.join(", ")}`, + violations, + }; } From 49840e1efeeae756f4bd8a10737b995f9a0ad69f Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:33:48 +0800 Subject: [PATCH 07/24] feat(api): accept mode on POST /skills/generate (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Callers can now choose the package shape: `mode: "simple" | "advanced"` on both the JSON body (single-turn `prompt` and multi-turn `messages`) and the multipart form. Omitted or empty → `advanced`, i.e. exactly what every caller got before, so no existing agent integration changes behaviour. An unknown value — or a multipart `mode` that arrives as a file or a repeated field — fails with 400 `invalid_mode` naming the accepted values. The check runs before `preflight()`, so like the other body validations it can never strand a reserved quota slot (#808); the route tests assert that neither `resolveModel` nor `checkAllowed` is reached. `mode` is included in the request `info` log lines so a violation retry in the service log can be correlated with what the caller asked for. `from-source` / `from-openapi` are unchanged: they always produce a plain, file-less skill and do not take `mode`. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/skills/generation/routes.test.ts | 134 +++++++++++++++++- .../src/domains/skills/generation/routes.ts | 51 ++++++- 2 files changed, 173 insertions(+), 12 deletions(-) diff --git a/ornn-api/src/domains/skills/generation/routes.test.ts b/ornn-api/src/domains/skills/generation/routes.test.ts index f9f41f52..f10c900f 100644 --- a/ornn-api/src/domains/skills/generation/routes.test.ts +++ b/ornn-api/src/domains/skills/generation/routes.test.ts @@ -38,7 +38,7 @@ import { createGenerationRoutes, type GenerationRoutesConfig } from "./routes"; import type { GenerateOptions } from "./service"; import { __resetRateLimitForTests } from "../../../middleware/rateLimit"; import { buildProblemJsonBody } from "../../../shared/types/index"; -import type { SkillStreamEvent } from "../../../shared/types/index"; +import type { GenerationMode, SkillStreamEvent } from "../../../shared/types/index"; import type { ChargeOutcome } from "../../quota/types"; import type { ModelResolution } from "../../settings/llmProviders/service"; @@ -83,14 +83,18 @@ interface ChargeCall { class FakeGenerationService { /** Frames every generate* method yields, in order. */ frames: SkillStreamEvent[] = happyFrames(); - generateStreamCalls: Array<{ query: string; modelOverride: string | undefined }> = []; + generateStreamCalls: Array<{ + query: string; + modelOverride: string | undefined; + mode: GenerationMode | undefined; + }> = []; fromOpenApiCalls: Array<{ spec: string }> = []; fromSourceCalls: Array<{ code: string; framework: string | undefined; sourceUrl: string | undefined; }> = []; - withHistoryCalls: Array<{ messages: unknown[] }> = []; + withHistoryCalls: Array<{ messages: unknown[]; mode: GenerationMode | undefined }> = []; private async *emit(): AsyncIterable { for (const f of this.frames) yield f; @@ -100,14 +104,19 @@ class FakeGenerationService { query: string, options: GenerateOptions = {}, ): AsyncIterable { - this.generateStreamCalls.push({ query, modelOverride: options.modelOverride }); + this.generateStreamCalls.push({ + query, + modelOverride: options.modelOverride, + mode: options.mode, + }); return this.emit(); } generateStreamWithHistory( messages: unknown[], + options: GenerateOptions = {}, ): AsyncIterable { - this.withHistoryCalls.push({ messages }); + this.withHistoryCalls.push({ messages, mode: options.mode }); return this.emit(); } @@ -428,6 +437,121 @@ describe("POST /skills/generate — preflight order", () => { }); }); +// ---- mode (#1242) ---------------------------------------------------- + +describe("POST /skills/generate — mode", () => { + it("defaults to advanced when the JSON body omits mode", async () => { + const gen = new FakeGenerationService(); + const { app } = buildApp({ generationService: gen }); + const res = await app.request("/api/v1/skills/generate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ prompt: "p" }), + }); + expect(res.status).toBe(200); + expect(gen.generateStreamCalls[0]!.mode).toBe("advanced"); + }); + + it("threads mode=simple from a single-turn JSON body", async () => { + const gen = new FakeGenerationService(); + const { app } = buildApp({ generationService: gen }); + const res = await app.request("/api/v1/skills/generate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ prompt: "p", mode: "simple" }), + }); + expect(res.status).toBe(200); + expect(gen.generateStreamCalls[0]!.mode).toBe("simple"); + }); + + it("threads mode=simple from a multi-turn JSON body", async () => { + const gen = new FakeGenerationService(); + const { app } = buildApp({ generationService: gen }); + const res = await app.request("/api/v1/skills/generate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ messages: [{ role: "user", content: "p" }], mode: "simple" }), + }); + expect(res.status).toBe(200); + expect(gen.withHistoryCalls[0]!.mode).toBe("simple"); + }); + + it("rejects an unknown mode with 400 invalid_mode BEFORE model resolution or quota reserve", async () => { + const gen = new FakeGenerationService(); + const quota = new FakeQuotaService(); + const providers = new FakeLlmProvidersService(); + const { app } = buildApp({ generationService: gen, quotaService: quota, llmProvidersService: providers }); + const res = await app.request("/api/v1/skills/generate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ prompt: "p", mode: "nope" }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { code: string; detail: string }; + expect(body.code).toBe("invalid_mode"); + expect(body.detail).toContain("simple, advanced"); + expect(providers.resolveModelArgs).toHaveLength(0); + expect(quota.checkAllowedCalls).toBe(0); + expect(quota.charges).toHaveLength(0); + expect(gen.generateStreamCalls).toHaveLength(0); + }); + + it("rejects a non-string mode (e.g. a number) with invalid_mode", async () => { + const { app } = buildApp(); + const res = await app.request("/api/v1/skills/generate", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ messages: [{ role: "user", content: "p" }], mode: 1 }), + }); + expect(res.status).toBe(400); + expect(((await res.json()) as { code: string }).code).toBe("invalid_mode"); + }); + + it("threads mode from a multipart form field", async () => { + const gen = new FakeGenerationService(); + const { app } = buildApp({ generationService: gen }); + const form = new FormData(); + form.set("prompt", "p"); + form.set("mode", "simple"); + const res = await app.request("/api/v1/skills/generate", { method: "POST", body: form }); + expect(res.status).toBe(200); + expect(gen.generateStreamCalls[0]!.mode).toBe("simple"); + }); + + it("treats an empty multipart mode field as the default", async () => { + const gen = new FakeGenerationService(); + const { app } = buildApp({ generationService: gen }); + const form = new FormData(); + form.set("prompt", "p"); + form.set("mode", ""); + const res = await app.request("/api/v1/skills/generate", { method: "POST", body: form }); + expect(res.status).toBe(200); + expect(gen.generateStreamCalls[0]!.mode).toBe("advanced"); + }); + + it("rejects an unknown multipart mode with 400 invalid_mode and no quota reserve", async () => { + const quota = new FakeQuotaService(); + const { app } = buildApp({ quotaService: quota }); + const form = new FormData(); + form.set("prompt", "p"); + form.set("mode", "ultra"); + const res = await app.request("/api/v1/skills/generate", { method: "POST", body: form }); + expect(res.status).toBe(400); + expect(((await res.json()) as { code: string }).code).toBe("invalid_mode"); + expect(quota.checkAllowedCalls).toBe(0); + }); + + it("rejects a multipart mode sent as a file with invalid_mode", async () => { + const { app } = buildApp(); + const form = new FormData(); + form.set("prompt", "p"); + form.set("mode", new Blob(["simple"], { type: "text/plain" }), "mode.txt"); + const res = await app.request("/api/v1/skills/generate", { method: "POST", body: form }); + expect(res.status).toBe(400); + expect(((await res.json()) as { code: string }).code).toBe("invalid_mode"); + }); +}); + // ---- Validation cases ------------------------------------------------ describe("POST /skills/generate — validation", () => { diff --git a/ornn-api/src/domains/skills/generation/routes.ts b/ornn-api/src/domains/skills/generation/routes.ts index e9965801..d816394b 100644 --- a/ornn-api/src/domains/skills/generation/routes.ts +++ b/ornn-api/src/domains/skills/generation/routes.ts @@ -14,7 +14,12 @@ import { requirePermission, getAuth, } from "../../../middleware/nyxidAuth"; -import { AppError } from "../../../shared/types/index"; +import { + AppError, + DEFAULT_GENERATION_MODE, + GENERATION_MODES, + type GenerationMode, +} from "../../../shared/types/index"; import { validateBody, getValidatedBody } from "../../../middleware/validate"; import { rateLimit } from "../../../middleware/rateLimit"; import { fetchGithubSourceBundle } from "./githubFetcher"; @@ -32,6 +37,28 @@ const logger = createLogger("skillGenerationRoutes"); */ const MAX_GENERATION_CHARS = 32_000; +const generationModeSchema = z.enum(GENERATION_MODES); + +/** + * Parse the optional `mode` field shared by the JSON and multipart + * branches of `POST /skills/generate` (#1242). Absent / empty → the + * backward-compatible default. Anything else must be one of the known + * modes — a `File` or repeated form field is rejected the same way as an + * unknown string. Called BEFORE `preflight()` so a bad value is a plain + * 400 and never strands a reserved quota slot (#808). + */ +function parseGenerationMode(raw: unknown): GenerationMode { + if (raw === undefined || raw === null || raw === "") return DEFAULT_GENERATION_MODE; + const parsed = generationModeSchema.safeParse(raw); + if (!parsed.success) { + throw AppError.badRequest( + "invalid_mode", + `'mode' must be one of: ${GENERATION_MODES.join(", ")}`, + ); + } + return parsed.data; +} + export interface GenerationRoutesConfig { generationService: SkillGenerationService; /** @@ -55,7 +82,8 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V /** * POST /skills/generate - * Input: multipart (prompt + optional package ZIP) or JSON (prompt or messages, optional modelId) + * Input: multipart (prompt + optional package ZIP) or JSON (prompt or + * messages), each with optional modelId + mode (#1242) * Response: SSE stream of generation events * Requires: ornn:skill:build */ @@ -74,6 +102,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V let prompt: string; let packageContent: string | null = null; let requestedModelId: string | undefined; + let mode: GenerationMode; if (contentType.includes("multipart/form-data")) { const body = await c.req.parseBody({ all: true }); @@ -86,6 +115,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V if (typeof body["modelId"] === "string" && body["modelId"]) { requestedModelId = body["modelId"]; } + mode = parseGenerationMode(body["mode"]); const packageFile = body["package"]; if (packageFile instanceof File) { @@ -96,7 +126,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V // Hybrid endpoint — multipart-or-JSON. Inline Zod parse so // malformed JSON returns 400 invalid_body via the global RFC // 7807 handler instead of a raw SyntaxError 500 (#438). - let body: { modelId?: string; messages?: unknown[]; prompt?: string }; + let body: { modelId?: string; messages?: unknown[]; prompt?: string; mode?: unknown }; try { const text = await c.req.text(); const raw = text.trim().length === 0 ? {} : JSON.parse(text); @@ -111,6 +141,7 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V if (typeof body.modelId === "string" && body.modelId) { requestedModelId = body.modelId; } + mode = parseGenerationMode(body.mode); // Multi-turn format: messages array if (body.messages && Array.isArray(body.messages)) { @@ -129,14 +160,17 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V ); } } - logger.info({ userId: authCtx.userId, messageCount: body.messages.length }, "Multi-turn generation request"); + logger.info( + { userId: authCtx.userId, messageCount: body.messages.length, mode }, + "Multi-turn generation request", + ); const pf = await preflight(c, quotaService, llmProvidersService, requestedModelId); const keepAliveMs = await resolveKeepAliveMs(keepAliveIntervalMsResolver); return streamGenerationEvents( c, generationService.generateStreamWithHistory( body.messages as Array<{ role: "user" | "assistant"; content: string }>, - { signal: c.req.raw.signal, modelOverride: pf.modelId }, + { signal: c.req.raw.signal, modelOverride: pf.modelId, mode }, ), keepAliveMs, { quotaService, userId: pf.userId, permissions: pf.permissions, modelId: pf.modelId, reservedAt: pf.reservedAt }, @@ -165,12 +199,15 @@ export function createGenerationRoutes(config: GenerationRoutesConfig): Hono<{ V ? `Existing skill package content:\n${packageContent}\n\nUser requirement: ${prompt}` : prompt; - logger.info({ userId: authCtx.userId, promptLength: prompt.length, modelId: pf.modelId }, "Generation request"); + logger.info( + { userId: authCtx.userId, promptLength: prompt.length, modelId: pf.modelId, mode }, + "Generation request", + ); const keepAliveMs = await resolveKeepAliveMs(keepAliveIntervalMsResolver); return streamGenerationEvents( c, - generationService.generateStream(query, { signal, modelOverride: pf.modelId }), + generationService.generateStream(query, { signal, modelOverride: pf.modelId, mode }), keepAliveMs, { quotaService, userId: pf.userId, permissions: pf.permissions, modelId: pf.modelId, reservedAt: pf.reservedAt }, ); From bfb1b60481792eff75476f39afa667e3711efa98 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:34:46 +0800 Subject: [PATCH 08/24] docs(api): document generation mode on the OpenAPI contract (#1242) The request bodies for `POST /skills/generate` are hand-written JSON Schema (the handler parses manually, so nothing derives them), which means `openapi.json` would have silently gone stale without this. - `mode` property (enum, default `advanced`, `invalid_mode` rejection, the exact simple-mode rule) on both the JSON and multipart bodies; the multipart variant also states that an attached `package` is still read as context in full even in simple mode. - `GENERATION_STREAM_CONTRACT` lists the new `references[]` / `assets[]` members of `raw` and how to assemble the package from it. - The operation description spells out the retry matrix per input shape, including the one multi-turn exception (a simple-mode violation retries once, then ends on `error`), and the cost contract wording covers that case. - `from-source` / `from-openapi` state that they are always the equivalent of `simple` and that a `mode` key is ignored, so agents do not try to send it there. Contract tests (route parity, description substance, examples, problem+json responses) pass. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- ornn-api/src/openapi/paths/generation.ts | 32 +++++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/ornn-api/src/openapi/paths/generation.ts b/ornn-api/src/openapi/paths/generation.ts index 1677c31a..85ff3e91 100644 --- a/ornn-api/src/openapi/paths/generation.ts +++ b/ornn-api/src/openapi/paths/generation.ts @@ -67,6 +67,20 @@ const modelIdProperty: JsonSchema = { examples: ["gpt-4.1-mini"], }; +/** + * Caller-chosen package shape (#1242). Shared by the JSON and multipart + * bodies; the semantics are documented once here and referenced from the + * operation description. + */ +const modeProperty: JsonSchema = { + type: "string", + enum: ["simple", "advanced"], + default: "advanced", + description: + "Package shape you want back. `advanced` (the default, and the pre-existing behaviour) lets the model decide between a `plain` and a `runtime-based` skill and emit `scripts[]`, plus optional `references[]` and `assets[]`. `simple` asks for a single `SKILL.md`: the model is told to keep everything inline, and the server rejects any answer that is not `category: \"plain\"` with empty `scripts`, `references`, `assets`, `runtimes`, `dependencies` and `envVars` — so `generation_complete.raw` in simple mode is guaranteed file-free. Omit or send an empty string for the default; any other value fails with 400 `invalid_mode` before model resolution or the quota reserve.", + examples: ["simple"], +}; + const generateJsonBody: JsonSchema = { type: "object", description: @@ -100,6 +114,7 @@ const generateJsonBody: JsonSchema = { }, }, modelId: modelIdProperty, + mode: modeProperty, }, }; @@ -118,6 +133,11 @@ const generateMultipartBody: JsonSchema = { type: "string", description: "Same semantics as the JSON body's `modelId`. Sent as a plain form field.", }, + mode: { + ...modeProperty, + description: + "Same semantics as the JSON body's `mode`. Sent as a plain form field — a file part or a repeated field under this name fails with 400 `invalid_mode`. Note that in `simple` mode an attached `package` is still read as context in full (including its `scripts/`), but the answer is still required to be `SKILL.md`-only.", + }, package: { type: "string", format: "binary", @@ -268,10 +288,10 @@ const playgroundChatBody: JsonSchema = { * operation object in isolation. */ const GENERATION_STREAM_CONTRACT = - "Frames are plain `data:` lines carrying a JSON object with a `type` field — there is no SSE `event:` line on payload frames, so dispatch on `type` and not on the parser's event name. Vocabulary: `generation_start` (LLM call opened), `token` (`content` = incremental text, emit-as-you-go), `validation_error` (`message`, `retrying`), `generation_complete` (`raw` = the model's full output), `error` (`message`, terminal). Separate keep-alive frames named `keepalive` with an empty payload arrive every `skillGen.sseKeepAliveMs` (admin-settable, 15 000 ms fallback) — ignore them. `raw` is a JSON **document string**, not a ZIP and not markdown: parse it to get `{ name, description, category, tags, readmeBody, runtimes, dependencies, envVars, scripts[], outputType? }`. Nothing is persisted — assemble the package yourself and `POST /api/v1/skills` to publish it."; + "Frames are plain `data:` lines carrying a JSON object with a `type` field — there is no SSE `event:` line on payload frames, so dispatch on `type` and not on the parser's event name. Vocabulary: `generation_start` (LLM call opened), `token` (`content` = incremental text, emit-as-you-go), `validation_error` (`message`, `retrying`), `generation_complete` (`raw` = the model's full output), `error` (`message`, terminal). Separate keep-alive frames named `keepalive` with an empty payload arrive every `skillGen.sseKeepAliveMs` (admin-settable, 15 000 ms fallback) — ignore them. `raw` is a JSON **document string**, not a ZIP and not markdown: parse it to get `{ name, description, category, tags, readmeBody, runtimes, dependencies, envVars, scripts[], references[], assets[], outputType? }` — `scripts`, `references` and `assets` are arrays of `{ filename, content }` text files destined for the folder of the same name (they are always present, empty when unused). Nothing is persisted — assemble the package yourself (`SKILL.md` = frontmatter built from the metadata + `readmeBody`) and `POST /api/v1/skills` to publish it."; const GENERATION_COST_CONTRACT = - "Requires the `ornn:skill:build` scope. One per-user monthly `skillGen` quota slot is reserved before the stream opens and reconciled when it ends, purely from what the stream emitted: the slot is consumed if and only if a `generation_complete` or a `validation_error` frame went out, and released in every other case. Two consequences worth designing for — a run that ends on `error` without a preceding `validation_error` costs nothing, and disconnecting mid-stream also costs nothing no matter how many `token` frames you already consumed (unlike `/playground/chat` and `/assistant/chat`, this surface has no abort-after-billable-output commit); conversely a single-turn run whose retry also fails validation consumes the slot even though you never received `generation_complete`. Model resolution and the quota check both run before the first byte, so their failures are ordinary JSON errors — never a truncated stream."; + "Requires the `ornn:skill:build` scope. One per-user monthly `skillGen` quota slot is reserved before the stream opens and reconciled when it ends, purely from what the stream emitted: the slot is consumed if and only if a `generation_complete` or a `validation_error` frame went out, and released in every other case. Two consequences worth designing for — a run that ends on `error` without a preceding `validation_error` costs nothing, and disconnecting mid-stream also costs nothing no matter how many `token` frames you already consumed (unlike `/playground/chat` and `/assistant/chat`, this surface has no abort-after-billable-output commit); conversely a run whose retry also fails validation (single-turn, or a simple-mode violation on either path) consumes the slot even though you never received `generation_complete`. Model resolution and the quota check both run before the first byte, so their failures are ordinary JSON errors — never a truncated stream."; function generateOperation(): Record { return { @@ -279,7 +299,8 @@ function generateOperation(): Record { description: "Streams an LLM-authored skill package from a natural-language brief. This is the front door of the generation family; use `/skills/generate/from-source` when you already have backend code and `/skills/generate/from-openapi` when you already have a spec. " + "The endpoint is hybrid on `Content-Type`. With `application/json` you send either `prompt` (single-turn) or `messages` (multi-turn refinement — resend the whole transcript, the server is stateless); with `multipart/form-data` you send a `prompt` field plus an optional `package` ZIP whose text files are read and prepended as context, which is how you ask for a modification of an existing skill rather than a fresh one. Any other content type is rejected with 400 `invalid_content_type`. " + - "Retry behaviour differs by mode and is worth handling explicitly: the single-turn `prompt` path re-asks the model once when the first answer is not valid JSON (you see `validation_error` with `retrying: true`) and may then end on `error` with no `generation_complete` at all, while the multi-turn `messages` path does not retry — it emits `validation_error` with `retrying: false` and still emits `generation_complete` carrying output that failed validation, so re-validate `raw` before trusting it. " + + "`mode` picks the package shape: `advanced` (default) may return `scripts[]`, `references[]` and `assets[]`; `simple` returns a `SKILL.md`-only skill and the server enforces it — see the `mode` property for the exact rule. " + + "Retry behaviour differs by input shape and is worth handling explicitly. The single-turn `prompt` path re-asks the model once whenever the first answer is rejected — not valid JSON, schema-invalid, or (in `simple` mode) carrying files — you see `validation_error` with `retrying: true` and the run may then end on `error` with no `generation_complete` at all. The multi-turn `messages` path does not retry a merely non-JSON answer (a refinement turn may legitimately be prose): it emits `validation_error` with `retrying: false` and still emits `generation_complete` carrying that output, so re-validate `raw` before trusting it. The one multi-turn exception is a `simple`-mode violation, which gets the same single corrective retry and ends on `error` if the model still emits files — `generation_complete.raw` in `simple` mode never carries scripts, references or assets. " + GENERATION_STREAM_CONTRACT + " " + GENERATION_COST_CONTRACT + @@ -298,6 +319,7 @@ function generateOperation(): Record { example: { prompt: "Build a skill that extracts tables from a PDF and returns them as CSV.", modelId: "gpt-4.1-mini", + mode: "simple", }, }, "multipart/form-data": { schema: generateMultipartBody }, @@ -311,7 +333,7 @@ function generateOperation(): Record { ...problemResponses( { 400: - "Rejected before the stream opened. Codes: `invalid_content_type` (neither JSON nor multipart), `invalid_body` (unparseable JSON, or valid JSON that is an array or a scalar rather than an object), `missing_prompt` (no usable `prompt` — an empty body lands here, since it is read as `{}`), `prompt_too_long` / `content_too_long` (over 32 000 characters), `MODEL_NOT_FOUND` / `MODEL_NOT_ENABLED` (the `modelId` you asked for is unknown, or not enabled for the `skillGen` surface).", + "Rejected before the stream opened. Codes: `invalid_content_type` (neither JSON nor multipart), `invalid_body` (unparseable JSON, or valid JSON that is an array or a scalar rather than an object), `missing_prompt` (no usable `prompt` — an empty body lands here, since it is read as `{}`), `prompt_too_long` / `content_too_long` (over 32 000 characters), `invalid_mode` (`mode` is not `simple` or `advanced`), `MODEL_NOT_FOUND` / `MODEL_NOT_ENABLED` (the `modelId` you asked for is unknown, or not enabled for the `skillGen` surface).", }, 401, { 403: "`forbidden` — the token authenticated but carries no `ornn:skill:build` scope. Generation is a high-cost surface and is gated separately from ordinary skill reads." }, @@ -336,6 +358,7 @@ function generateFromSourceOperation(): Record { "Turns existing backend code into a `plain` (documentation-only, no runtime scripts) skill that teaches an agent how to call that service. Supply the code inline via `code`, or hand over a public GitHub URL via `repoUrl` and let the server harvest it — exactly one of the two, never both. " + "The harvester is deliberately small: it walks one directory (`path`, else the URL's `/tree/{ref}/{subpath}`, else a list of conventional route folders), takes at most 8 source files of at most 16 KiB each, concatenates them with `// FILE: ` markers, and infers a framework hint. Every failure mode of that fetch — not a GitHub URL, private repository, missing directory, anonymous rate limit exhausted — collapses into a single 400 `repo_fetch_failed` whose `detail` carries the underlying reason. For anything larger or non-public, fetch the files yourself and pass them as `code`. " + "Unlike the prompt-driven endpoint this path never retries: a model answer that fails schema validation produces `validation_error` with `retrying: false` and is still delivered in the following `generation_complete`, so validate `raw` yourself before publishing. " + + "There is no `mode` here — the output is always the equivalent of `simple` (a `plain` skill with empty `scripts`, `references` and `assets`); a `mode` key in the body is ignored, not rejected. " + GENERATION_STREAM_CONTRACT + " " + GENERATION_COST_CONTRACT + @@ -382,6 +405,7 @@ function generateFromOpenApiOperation(): Record { "Converts an OpenAPI document into a `plain` (documentation-only) skill that teaches an agent how to call the described API. This is the highest-fidelity member of the generation family — prefer it over `/skills/generate/from-source` whenever a spec exists, because the model reads declared schemas instead of inferring them from handler code. " + "`spec` is the document as a raw string (JSON or YAML) and is inlined verbatim into the prompt, so it competes with the model's context budget: for a large surface, narrow it with `endpoints` (an allow-list of `METHOD /path` strings) or pre-trim the document. `description` adds context the spec cannot express, such as how to obtain credentials. " + "Like the from-source path this never retries — a schema-invalid answer yields `validation_error` with `retrying: false` and is still delivered in `generation_complete`, so re-validate `raw` before you publish it. " + + "There is no `mode` here either — the output is always the equivalent of `simple` (a `plain` skill with empty `scripts`, `references` and `assets`); a `mode` key in the body is ignored, not rejected. " + GENERATION_STREAM_CONTRACT + " " + GENERATION_COST_CONTRACT + From 660078aa6f192b779351b5c8ce26b70fcd5ff8c2 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:37:40 +0800 Subject: [PATCH 09/24] refactor(web): decompose CreateSkillGenerativePage (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page was 591 lines — over the 500-line file limit — before the mode toggle (#1242) adds to its composer row. Pure extraction, markup and behaviour unchanged: - `hooks/useGenerativeDrawer` owns the hover / pin / Esc state and the new-iteration hint. The two `setState`-in-effect transitions became the "adjust state during render" guard the react-hooks lint wants (same fix as #888 in ModelPicker) — the extracted hook tripped the rule where the inlined page had not. Now covered by its own test. - `components/skill/generative/GenerativeEmptyHero` — eyebrow + headline + the three prompt starters (the starters were a module-level helper on the page; they are static copy so they live with the hero now). - `components/skill/generative/GenerativePackageRailTab` — the right-edge tab with its hint rings / tooltip / warning dot. The page drops to 403 lines. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../skill/generative/GenerativeEmptyHero.tsx | 102 ++++++++ .../generative/GenerativePackageRailTab.tsx | 109 ++++++++ .../src/hooks/useGenerativeDrawer.test.tsx | 118 +++++++++ ornn-web/src/hooks/useGenerativeDrawer.ts | 109 ++++++++ .../pages/skill/CreateSkillGenerativePage.tsx | 240 ++---------------- 5 files changed, 464 insertions(+), 214 deletions(-) create mode 100644 ornn-web/src/components/skill/generative/GenerativeEmptyHero.tsx create mode 100644 ornn-web/src/components/skill/generative/GenerativePackageRailTab.tsx create mode 100644 ornn-web/src/hooks/useGenerativeDrawer.test.tsx create mode 100644 ornn-web/src/hooks/useGenerativeDrawer.ts diff --git a/ornn-web/src/components/skill/generative/GenerativeEmptyHero.tsx b/ornn-web/src/components/skill/generative/GenerativeEmptyHero.tsx new file mode 100644 index 00000000..2977ed77 --- /dev/null +++ b/ornn-web/src/components/skill/generative/GenerativeEmptyHero.tsx @@ -0,0 +1,102 @@ +/** + * Empty-state hero for CreateSkillGenerativePage (#1242 decomposition). + * + * Centered welcome flag — eyebrow + headline + lead sentence + three + * prompt-starter chips + a drawer-discovery footer note. Mirrors + * PlaygroundEmptyHero so the two chat surfaces read as one family. + * + * Stateless — the parent owns the click handler; the starters are + * localized here because they are static copy. + * + * @module components/skill/generative/GenerativeEmptyHero + */ + +import { useTranslation } from "react-i18next"; + +export interface PromptStarter { + label: string; + body: string; +} + +type TFunc = ReturnType["t"]; + +function defaultPromptStarters(t: TFunc): PromptStarter[] { + return [ + { + label: t("generative.starter1Label", "Slack notifier"), + body: t( + "generative.starter1Body", + "Build a skill that posts a formatted message to a Slack channel via webhook. Take channel + message as inputs.", + ), + }, + { + label: t("generative.starter2Label", "Fetch GitHub PRs"), + body: t( + "generative.starter2Body", + "Build a skill that lists open pull requests for a given GitHub repo, sorted by latest activity.", + ), + }, + { + label: t("generative.starter3Label", "CSV → JSON"), + body: t( + "generative.starter3Body", + "Build a skill that reads a CSV file and outputs a JSON array, inferring types per column.", + ), + }, + ]; +} + +export interface GenerativeEmptyHeroProps { + onStarterClick: (body: string) => void; +} + +export function GenerativeEmptyHero({ onStarterClick }: GenerativeEmptyHeroProps) { + const { t } = useTranslation(); + const starters = defaultPromptStarters(t); + + return ( +
+
+
+
+ {t("generative.eyebrow", "Generative skill builder")} +
+

+ {t("generative.heroTitle", "Describe a skill. Build it.")} +

+

+ {t( + "generative.heroSubtitle", + "Tell the model what the skill should do. It drafts the package; you iterate; you save.", + )} +

+
+ +
+ {starters.map((s) => ( + + ))} +
+ +

+ {t( + "generative.drawerHint", + "Package preview + Save on the right edge", + )} +

+
+
+ ); +} diff --git a/ornn-web/src/components/skill/generative/GenerativePackageRailTab.tsx b/ornn-web/src/components/skill/generative/GenerativePackageRailTab.tsx new file mode 100644 index 00000000..c3048f95 --- /dev/null +++ b/ornn-web/src/components/skill/generative/GenerativePackageRailTab.tsx @@ -0,0 +1,109 @@ +/** + * Right-edge rail tab that opens the package drawer on the generative + * page (#1242 decomposition of CreateSkillGenerativePage). + * + * Single tab (Package + actions). Carries three overlays: + * - the new-iteration hint — pulsing ember rings + an ember dot when a + * generation lands while the drawer is closed; + * - a horizontal `[§ PACKAGE]` tooltip on hover while closed; + * - a warning dot when the previewed SKILL.md has frontmatter errors. + * + * Stateless — the parent's `useGenerativeDrawer` owns open/pin state. + * + * @module components/skill/generative/GenerativePackageRailTab + */ + +import { useTranslation } from "react-i18next"; +import { PackageIcon } from "@/components/icons"; + +export interface GenerativePackageRailTabProps { + drawerOpen: boolean; + pinnedOpen: boolean; + hasUnseenIteration: boolean; + hasFrontmatterErrors: boolean; + onHoverOpen: () => void; + onHoverCloseScheduled: () => void; + onTogglePin: () => void; +} + +export function GenerativePackageRailTab({ + drawerOpen, + pinnedOpen, + hasUnseenIteration, + hasFrontmatterErrors, + onHoverOpen, + onHoverCloseScheduled, + onTogglePin, +}: GenerativePackageRailTabProps) { + const { t } = useTranslation(); + + return ( +
+ +
+ ); +} diff --git a/ornn-web/src/hooks/useGenerativeDrawer.test.tsx b/ornn-web/src/hooks/useGenerativeDrawer.test.tsx new file mode 100644 index 00000000..232d6fed --- /dev/null +++ b/ornn-web/src/hooks/useGenerativeDrawer.test.tsx @@ -0,0 +1,118 @@ +/** + * UT-WEB-GENERATIVE-DRAWER-001 (#1242) + * + * Pins the drawer state machine extracted from CreateSkillGenerativePage: + * pinned-open by default, hover open / delayed close, Esc unpins, and + * the new-iteration hint that flips on the `generating → preview` edge + * only while the drawer is closed and clears as soon as it opens. + * + * @module hooks/useGenerativeDrawer.test + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { useGenerativeDrawer } from "./useGenerativeDrawer"; +import type { GenerationPhase } from "@/types/skillPackage"; + +describe("useGenerativeDrawer", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it("starts pinned open", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + expect(result.current.pinnedOpen).toBe(true); + expect(result.current.drawerOpen).toBe(true); + }); + + it("togglePin closes a pinned drawer and re-opens it", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + act(() => result.current.togglePin()); + expect(result.current.pinnedOpen).toBe(false); + expect(result.current.drawerOpen).toBe(false); + act(() => result.current.togglePin()); + expect(result.current.drawerOpen).toBe(true); + }); + + it("hover opens an unpinned drawer and closes after the delay", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + act(() => result.current.unpin()); + expect(result.current.drawerOpen).toBe(false); + + act(() => result.current.openHover()); + expect(result.current.drawerOpen).toBe(true); + + act(() => result.current.scheduleHoverClose()); + // Still open until the close delay elapses. + expect(result.current.drawerOpen).toBe(true); + act(() => { + vi.advanceTimersByTime(300); + }); + expect(result.current.drawerOpen).toBe(false); + }); + + it("re-entering during the close delay cancels the pending close", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + act(() => result.current.unpin()); + act(() => result.current.openHover()); + act(() => result.current.scheduleHoverClose()); + act(() => result.current.openHover()); + act(() => { + vi.advanceTimersByTime(300); + }); + expect(result.current.drawerOpen).toBe(true); + }); + + it("close() clears both pin and hover state", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + act(() => result.current.openHover()); + act(() => result.current.close()); + expect(result.current.pinnedOpen).toBe(false); + expect(result.current.drawerOpen).toBe(false); + }); + + it("Escape unpins a pinned drawer", () => { + const { result } = renderHook(() => useGenerativeDrawer("input")); + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })); + }); + expect(result.current.pinnedOpen).toBe(false); + }); + + it("flags an unseen iteration only when a generation lands while closed, and clears on open", () => { + const { result, rerender } = renderHook( + ({ phase }: { phase: GenerationPhase }) => useGenerativeDrawer(phase), + { initialProps: { phase: "input" as GenerationPhase } }, + ); + + // Drawer open (default): a landed generation is NOT unseen. + rerender({ phase: "generating" }); + rerender({ phase: "preview" }); + expect(result.current.hasUnseenIteration).toBe(false); + + // Close, then run another generation → flagged. + act(() => result.current.unpin()); + rerender({ phase: "generating" }); + rerender({ phase: "preview" }); + expect(result.current.hasUnseenIteration).toBe(true); + + // Opening the drawer clears the hint. + act(() => result.current.togglePin()); + expect(result.current.drawerOpen).toBe(true); + expect(result.current.hasUnseenIteration).toBe(false); + }); + + it("does not flag an error transition as an unseen iteration", () => { + const { result, rerender } = renderHook( + ({ phase }: { phase: GenerationPhase }) => useGenerativeDrawer(phase), + { initialProps: { phase: "input" as GenerationPhase } }, + ); + act(() => result.current.unpin()); + rerender({ phase: "generating" }); + rerender({ phase: "error" }); + expect(result.current.hasUnseenIteration).toBe(false); + }); +}); diff --git a/ornn-web/src/hooks/useGenerativeDrawer.ts b/ornn-web/src/hooks/useGenerativeDrawer.ts new file mode 100644 index 00000000..e38ea818 --- /dev/null +++ b/ornn-web/src/hooks/useGenerativeDrawer.ts @@ -0,0 +1,109 @@ +/** + * Drawer state for the generative skill builder (#1242 decomposition of + * CreateSkillGenerativePage). + * + * Same hover / pin primitive as the playground drawer, but the package + * drawer is **pinned open by default** because the preview IS the work + * product. Also owns the "new iteration" hint: the chat lets the user + * refine across many turns, so each `generating → preview` transition + * produces a fresh package; when that lands while the drawer is closed + * the rail tab pulses so the user notices without scrolling. + * + * @module hooks/useGenerativeDrawer + */ + +import { useCallback, useEffect, useRef, useState } from "react"; +import type { GenerationPhase } from "@/types/skillPackage"; + +/** Delay before a hover-opened drawer closes after the pointer leaves. */ +const HOVER_CLOSE_DELAY_MS = 220; + +export interface UseGenerativeDrawerReturn { + /** True when either pinned or hover-opened. */ + drawerOpen: boolean; + pinnedOpen: boolean; + /** A generation landed while the drawer was closed and hasn't been seen. */ + hasUnseenIteration: boolean; + openHover: () => void; + scheduleHoverClose: () => void; + togglePin: () => void; + /** Close regardless of how it was opened. */ + close: () => void; + unpin: () => void; +} + +export function useGenerativeDrawer(phase: GenerationPhase): UseGenerativeDrawerReturn { + const [hoverDrawerOpen, setHoverDrawerOpen] = useState(false); + const [pinnedOpen, setPinnedOpen] = useState(true); + const closeTimerRef = useRef | null>(null); + + const openHover = useCallback(() => { + if (closeTimerRef.current) { + clearTimeout(closeTimerRef.current); + closeTimerRef.current = null; + } + setHoverDrawerOpen(true); + }, []); + + const scheduleHoverClose = useCallback(() => { + if (closeTimerRef.current) clearTimeout(closeTimerRef.current); + closeTimerRef.current = setTimeout(() => { + setHoverDrawerOpen(false); + closeTimerRef.current = null; + }, HOVER_CLOSE_DELAY_MS); + }, []); + + const togglePin = useCallback(() => { + setPinnedOpen((cur) => !cur); + setHoverDrawerOpen(false); + }, []); + + const close = useCallback(() => { + setPinnedOpen(false); + setHoverDrawerOpen(false); + }, []); + + const unpin = useCallback(() => setPinnedOpen(false), []); + + // Esc closes a pinned drawer. + useEffect(() => { + if (!pinnedOpen) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") setPinnedOpen(false); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [pinnedOpen]); + + const drawerOpen = pinnedOpen || hoverDrawerOpen; + + // Both transitions use the "adjust state during render" guard rather + // than a setState inside an effect (avoids the cascading render the + // react-hooks lint flags, #888): the flag flips on the + // `generating → preview` edge while closed, and clears on the + // `closed → open` edge. + const [hasUnseenIteration, setHasUnseenIteration] = useState(false); + const [prevPhase, setPrevPhase] = useState(phase); + if (phase !== prevPhase) { + setPrevPhase(phase); + if (prevPhase === "generating" && phase === "preview" && !drawerOpen) { + setHasUnseenIteration(true); + } + } + const [prevDrawerOpen, setPrevDrawerOpen] = useState(drawerOpen); + if (drawerOpen !== prevDrawerOpen) { + setPrevDrawerOpen(drawerOpen); + if (drawerOpen) setHasUnseenIteration(false); + } + + return { + drawerOpen, + pinnedOpen, + hasUnseenIteration, + openHover, + scheduleHoverClose, + togglePin, + close, + unpin, + }; +} diff --git a/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx b/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx index ea376c2a..4a98438b 100644 --- a/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx +++ b/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx @@ -27,10 +27,12 @@ import { ChatInput, type ChatInputHandle } from "@/components/playground/ChatInp import { SkillPackagePreview } from "@/components/skill/SkillPackagePreview"; import { ValidationErrorPanel } from "@/components/skill/ValidationErrorPanel"; import { GenerationChatMessage } from "@/components/skill/GenerationChatMessage"; +import { GenerativeEmptyHero } from "@/components/skill/generative/GenerativeEmptyHero"; +import { GenerativePackageRailTab } from "@/components/skill/generative/GenerativePackageRailTab"; import { ModelPicker } from "@/components/models/ModelPicker"; import { OverLimitPage } from "@/components/quota/OverLimitPage"; import { QuotaInline } from "@/components/quota/QuotaInline"; -import { PackageIcon } from "@/components/icons"; +import { useGenerativeDrawer } from "@/hooks/useGenerativeDrawer"; import { useSkillGeneration } from "@/hooks/useSkillGeneration"; import { useCreateSkill } from "@/hooks/useSkills"; import { useMyQuota } from "@/hooks/useQuota"; @@ -56,39 +58,6 @@ function WeldedSeam({ className = "" }: { className?: string }) { ); } -interface PromptStarter { - label: string; - body: string; -} - -type TFunc = ReturnType["t"]; - -function defaultPromptStarters(t: TFunc): PromptStarter[] { - return [ - { - label: t("generative.starter1Label", "Slack notifier"), - body: t( - "generative.starter1Body", - "Build a skill that posts a formatted message to a Slack channel via webhook. Take channel + message as inputs.", - ), - }, - { - label: t("generative.starter2Label", "Fetch GitHub PRs"), - body: t( - "generative.starter2Body", - "Build a skill that lists open pull requests for a given GitHub repo, sorted by latest activity.", - ), - }, - { - label: t("generative.starter3Label", "CSV → JSON"), - body: t( - "generative.starter3Body", - "Build a skill that reads a CSV file and outputs a JSON array, inferring types per column.", - ), - }, - ]; -} - export function CreateSkillGenerativePage() { const { t } = useTranslation(); const navigate = useNavigate(); @@ -186,69 +155,15 @@ export function CreateSkillGenerativePage() { } }; - // ── Drawer state — same primitive as the playground, but the drawer - // for the generative artifact is pinned-open by default since the - // preview IS the work product. - const [hoverDrawerOpen, setHoverDrawerOpen] = useState(false); - const [pinnedOpen, setPinnedOpen] = useState(true); - const closeTimerRef = useRef | null>(null); - const openHover = useCallback(() => { - if (closeTimerRef.current) { - clearTimeout(closeTimerRef.current); - closeTimerRef.current = null; - } - setHoverDrawerOpen(true); - }, []); - const scheduleHoverClose = useCallback(() => { - if (closeTimerRef.current) clearTimeout(closeTimerRef.current); - closeTimerRef.current = setTimeout(() => { - setHoverDrawerOpen(false); - closeTimerRef.current = null; - }, 220); - }, []); - const togglePin = useCallback(() => { - setPinnedOpen((cur) => !cur); - setHoverDrawerOpen(false); - }, []); - - // Esc closes a pinned drawer. - useEffect(() => { - if (!pinnedOpen) return; - const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") setPinnedOpen(false); - }; - window.addEventListener("keydown", onKey); - return () => window.removeEventListener("keydown", onKey); - }, [pinnedOpen]); + // ── Drawer state — hover / pin / esc / new-iteration hint live in + // the hook; the drawer for the generative artifact is pinned-open by + // default since the preview IS the work product. + const drawer = useGenerativeDrawer(generation.phase); const isGenerating = generation.phase === "generating"; const hasMessages = generation.chatMessages.length > 0; const hasPreview = generation.metadata !== null; const conversationActive = hasMessages || isGenerating; - const drawerOpen = pinnedOpen || hoverDrawerOpen; - - // New-iteration hint — pulse the rail tab when a generation lands while - // the drawer is closed. The chat lets the user refine across many turns, - // so each `phase: generating → preview` transition produces a fresh skill - // package; without this nudge the only signal is the chat message itself, - // which the user may scroll past while typing the next refinement. - const [hasUnseenIteration, setHasUnseenIteration] = useState(false); - const prevPhaseRef = useRef(generation.phase); - useEffect(() => { - if ( - prevPhaseRef.current === "generating" && - generation.phase === "preview" && - !drawerOpen - ) { - setHasUnseenIteration(true); - } - prevPhaseRef.current = generation.phase; - }, [generation.phase, drawerOpen]); - useEffect(() => { - if (drawerOpen) setHasUnseenIteration(false); - }, [drawerOpen]); - - const starters = defaultPromptStarters(t); const chatInputPlaceholder = isGenerating ? t("generative.placeholder") @@ -313,49 +228,7 @@ export function CreateSkillGenerativePage() {
{!conversationActive ? ( /* ─── Empty-state hero ─── */ -
-
-
-
- {t("generative.eyebrow", "Generative skill builder")} -
-

- {t("generative.heroTitle", "Describe a skill. Build it.")} -

-

- {t( - "generative.heroSubtitle", - "Tell the model what the skill should do. It drafts the package; you iterate; you save.", - )} -

-
- -
- {starters.map((s) => ( - - ))} -
- -

- {t( - "generative.drawerHint", - "Package preview + Save on the right edge", - )} -

-
-
+ ) : ( /* ─── Conversation ─── */
@@ -389,85 +262,27 @@ export function CreateSkillGenerativePage() { {/* ─── Right-edge rail — single tab (Package + actions) ─── */} -
- -
+ {/* ─── Drawer overlay ─── */} - {drawerOpen && ( + {drawer.drawerOpen && ( <> - {pinnedOpen && ( + {drawer.pinnedOpen && ( setPinnedOpen(false)} + onClick={drawer.unpin} className="fixed inset-0 z-30 bg-page/30 backdrop-blur-[1px]" /> )} @@ -477,8 +292,8 @@ export function CreateSkillGenerativePage() { animate={{ x: 0 }} exit={{ x: "100%" }} transition={{ duration: 0.18, ease: "easeOut" }} - onMouseEnter={openHover} - onMouseLeave={scheduleHoverClose} + onMouseEnter={drawer.openHover} + onMouseLeave={drawer.scheduleHoverClose} className="card-impression fixed right-10 top-[68px] bottom-4 z-40 flex w-[min(960px,65vw)] max-w-[calc(100vw-3rem)] flex-col rounded-md border border-subtle bg-card" role="complementary" aria-label={t("aria.skillPackagePreview")} @@ -489,7 +304,7 @@ export function CreateSkillGenerativePage() { [§ PACKAGE] - {pinnedOpen && ( + {drawer.pinnedOpen && ( {t("generative.pinned", "Pinned")} @@ -498,19 +313,16 @@ export function CreateSkillGenerativePage() {
+ ); + })} +
+
+ ); +} diff --git a/ornn-web/src/hooks/useGenerationMode.test.ts b/ornn-web/src/hooks/useGenerationMode.test.ts new file mode 100644 index 00000000..835fe2b5 --- /dev/null +++ b/ornn-web/src/hooks/useGenerationMode.test.ts @@ -0,0 +1,112 @@ +/** + * UT-WEB-GENERATION-MODE-PREF-001 (#1242) + * + * Pins the localStorage-backed mode preference: default `advanced`, + * round-trip through storage, rejection of unknown stored values, + * cross-tab `storage` sync, and tolerance of an unavailable storage — + * plus the shared label/hint copy the toggle and composer hint read. + * + * @module hooks/useGenerationMode.test + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { + GENERATION_MODE_STORAGE_KEY, + useGenerationModeCopy, + usePreferredGenerationMode, +} from "./useGenerationMode"; + +// jsdom here ships no working localStorage (see AnnouncementBanner.test), +// so install a minimal in-memory one. `throwing` flips every call into +// an exception to model a private-mode browser. +const store = new Map(); +let throwing = false; +function guard(fn: () => T): T { + if (throwing) throw new Error("storage blocked"); + return fn(); +} +const fake: Storage = { + get length() { + return store.size; + }, + clear: () => guard(() => store.clear()), + getItem: (k) => guard(() => (store.has(k) ? (store.get(k) as string) : null)), + key: (i) => Array.from(store.keys())[i] ?? null, + removeItem: (k) => guard(() => void store.delete(k)), + setItem: (k, v) => guard(() => void store.set(k, String(v))), +}; +Object.defineProperty(globalThis, "localStorage", { value: fake, configurable: true }); + +beforeEach(() => { + store.clear(); + throwing = false; +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("usePreferredGenerationMode", () => { + it("defaults to advanced when nothing is stored", () => { + const { result } = renderHook(() => usePreferredGenerationMode()); + expect(result.current[0]).toBe("advanced"); + }); + + it("reads a stored simple preference", () => { + window.localStorage.setItem(GENERATION_MODE_STORAGE_KEY, "simple"); + const { result } = renderHook(() => usePreferredGenerationMode()); + expect(result.current[0]).toBe("simple"); + }); + + it("falls back to advanced for an unknown stored value", () => { + window.localStorage.setItem(GENERATION_MODE_STORAGE_KEY, "ultra"); + const { result } = renderHook(() => usePreferredGenerationMode()); + expect(result.current[0]).toBe("advanced"); + }); + + it("setMode updates state and persists", () => { + const { result } = renderHook(() => usePreferredGenerationMode()); + act(() => result.current[1]("simple")); + expect(result.current[0]).toBe("simple"); + expect(window.localStorage.getItem(GENERATION_MODE_STORAGE_KEY)).toBe("simple"); + }); + + it("follows a storage event from another tab, ignoring other keys", () => { + const { result } = renderHook(() => usePreferredGenerationMode()); + act(() => { + window.dispatchEvent( + new StorageEvent("storage", { key: GENERATION_MODE_STORAGE_KEY, newValue: "simple" }), + ); + }); + expect(result.current[0]).toBe("simple"); + act(() => { + window.dispatchEvent(new StorageEvent("storage", { key: "unrelated", newValue: "x" })); + }); + expect(result.current[0]).toBe("simple"); + // A cleared / garbage value from another tab resets to the default. + act(() => { + window.dispatchEvent( + new StorageEvent("storage", { key: GENERATION_MODE_STORAGE_KEY, newValue: null }), + ); + }); + expect(result.current[0]).toBe("advanced"); + }); + + it("keeps working when storage throws (private mode)", () => { + throwing = true; + const { result } = renderHook(() => usePreferredGenerationMode()); + expect(result.current[0]).toBe("advanced"); + act(() => result.current[1]("simple")); + expect(result.current[0]).toBe("simple"); + }); +}); + +describe("useGenerationModeCopy", () => { + it("exposes the label + hint strings the toggle and composer hint render", () => { + const { result } = renderHook(() => useGenerationModeCopy()); + expect(result.current.labels).toEqual({ simple: "Simple", advanced: "Advanced" }); + expect(result.current.hints.simple).toBe("SKILL.md only — no scripts, references or assets"); + expect(result.current.hints.advanced).toBe("SKILL.md plus scripts, references and assets"); + }); +}); diff --git a/ornn-web/src/hooks/useGenerationMode.ts b/ornn-web/src/hooks/useGenerationMode.ts new file mode 100644 index 00000000..ddca5f6a --- /dev/null +++ b/ornn-web/src/hooks/useGenerationMode.ts @@ -0,0 +1,83 @@ +/** + * Preferred generation mode for the generative skill builder (#1242). + * + * `localStorage`-backed like `usePreferredModel` so the toggle lands on + * the user's last choice after a reload. The stored value is validated + * against `GENERATION_MODES` — an unknown or missing value falls back to + * `advanced`, which is also the server default, so a fresh browser and + * an omitted field mean the same thing. + * + * @module hooks/useGenerationMode + */ + +import { useCallback, useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { GENERATION_MODES, type GenerationMode } from "@/types/skillPackage"; + +export const GENERATION_MODE_STORAGE_KEY = "ornn.preferredMode.skillGen"; + +/** Matches the server's `DEFAULT_GENERATION_MODE`. */ +export const DEFAULT_GENERATION_MODE: GenerationMode = "advanced"; + +function isGenerationMode(value: unknown): value is GenerationMode { + return typeof value === "string" && (GENERATION_MODES as readonly string[]).includes(value); +} + +function readStoredMode(): GenerationMode { + if (typeof window === "undefined") return DEFAULT_GENERATION_MODE; + try { + const raw = window.localStorage.getItem(GENERATION_MODE_STORAGE_KEY); + return isGenerationMode(raw) ? raw : DEFAULT_GENERATION_MODE; + } catch { + return DEFAULT_GENERATION_MODE; + } +} + +export function usePreferredGenerationMode(): [GenerationMode, (mode: GenerationMode) => void] { + const [mode, setModeState] = useState(readStoredMode); + + // Sync if the user changes the preference in another tab. + useEffect(() => { + if (typeof window === "undefined") return; + const handler = (e: StorageEvent) => { + if (e.key === GENERATION_MODE_STORAGE_KEY) { + setModeState(isGenerationMode(e.newValue) ? e.newValue : DEFAULT_GENERATION_MODE); + } + }; + window.addEventListener("storage", handler); + return () => window.removeEventListener("storage", handler); + }, []); + + const setMode = useCallback((next: GenerationMode) => { + setModeState(next); + try { + window.localStorage.setItem(GENERATION_MODE_STORAGE_KEY, next); + } catch { + /* storage may be unavailable in private mode — ignore */ + } + }, []); + + return [mode, setMode]; +} + +/** + * Localized label + one-line description per mode. Shared by the + * toggle (segment text, `title`, aria) and the composer hint row so the + * copy cannot drift between the two. + */ +export function useGenerationModeCopy(): { + labels: Record; + hints: Record; +} { + const { t } = useTranslation(); + return { + labels: { + simple: t("generative.modeSimple", "Simple"), + advanced: t("generative.modeAdvanced", "Advanced"), + }, + hints: { + simple: t("generative.modeSimpleHint", "SKILL.md only — no scripts, references or assets"), + advanced: t("generative.modeAdvancedHint", "SKILL.md plus scripts, references and assets"), + }, + }; +} diff --git a/ornn-web/src/i18n/en.json b/ornn-web/src/i18n/en.json index af2a2473..6efad41f 100644 --- a/ornn-web/src/i18n/en.json +++ b/ornn-web/src/i18n/en.json @@ -604,7 +604,6 @@ "backToModes": "Back to mode selection", "title": "GENERATE SKILL", "desc": "Describe the skill you need and AI will generate it for you. You can refine it with follow-up messages.", - "note": "Generated skills are plain or runtime-based only.", "placeholder": "Generating...", "askPlaceholder": "Describe the skill you want to create…", "emptyPreview": "Generate a skill to preview its contents", @@ -619,6 +618,12 @@ "heroTitle": "Describe a skill. Build it.", "heroSubtitle": "Tell the model what the skill should do. It drafts the package; you iterate; you save.", "drawerHint": "Package preview + Save on the right edge", + "modeLabel": "Mode", + "modeAria": "Generation mode", + "modeSimple": "Simple", + "modeAdvanced": "Advanced", + "modeSimpleHint": "SKILL.md only — no scripts, references or assets", + "modeAdvancedHint": "SKILL.md plus scripts, references and assets", "tabPackage": "Package", "pin": "Pin", "unpin": "Unpin", diff --git a/ornn-web/src/i18n/generativeParity.test.ts b/ornn-web/src/i18n/generativeParity.test.ts new file mode 100644 index 00000000..09390b77 --- /dev/null +++ b/ornn-web/src/i18n/generativeParity.test.ts @@ -0,0 +1,69 @@ +/** + * i18n parity for the `generative` namespace (#1242). + * + * The global react-i18next test stub resolves keys against en.json only, + * so a key added to en.json but not zh.json passes every component test + * and only shows up as raw English in the zh UI. This pins the two + * locales to identical key sets for the generative page, the way + * skillsetParity.test.ts does for the skillset namespaces. + * + * @module i18n/generativeParity.test + */ + +import { describe, it, expect } from "vitest"; +import en from "./en.json"; +import zh from "./zh.json"; + +const NAMESPACE = "generative"; + +type Json = Record; + +/** Recursively flatten a nested object into dot-joined leaf keys. */ +function flatten(obj: Json, prefix = ""): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(obj)) { + const key = prefix ? `${prefix}.${k}` : k; + if (v && typeof v === "object" && !Array.isArray(v)) { + Object.assign(out, flatten(v as Json, key)); + } else { + out[key] = String(v); + } + } + return out; +} + +const enFlat = flatten(en as Json); +const zhFlat = flatten(zh as Json); + +function namespaceKeys(flat: Record): string[] { + return Object.keys(flat) + .filter((k) => k.startsWith(`${NAMESPACE}.`)) + .sort(); +} + +describe("generative i18n parity", () => { + it("has identical key sets in en + zh", () => { + expect(namespaceKeys(zhFlat)).toEqual(namespaceKeys(enFlat)); + }); + + it("carries the mode-toggle keys (#1242)", () => { + for (const key of [ + "modeLabel", + "modeAria", + "modeSimple", + "modeAdvanced", + "modeSimpleHint", + "modeAdvancedHint", + ]) { + expect(enFlat[`${NAMESPACE}.${key}`], `en ${key}`).toBeTruthy(); + expect(zhFlat[`${NAMESPACE}.${key}`], `zh ${key}`).toBeTruthy(); + } + }); + + it("no generative string is empty in either locale", () => { + for (const k of namespaceKeys(enFlat)) { + expect(enFlat[k]?.trim().length, `en ${k}`).toBeGreaterThan(0); + expect(zhFlat[k]?.trim().length, `zh ${k}`).toBeGreaterThan(0); + } + }); +}); diff --git a/ornn-web/src/i18n/zh.json b/ornn-web/src/i18n/zh.json index 615f563e..8f6cce86 100644 --- a/ornn-web/src/i18n/zh.json +++ b/ornn-web/src/i18n/zh.json @@ -604,7 +604,6 @@ "backToModes": "返回模式选择", "title": "AI 生成技能", "desc": "描述你需要的技能，AI 会为你生成，之后可以通过对话继续完善。", - "note": "生成的技能只支持纯文档或基于运行时的形态。", "placeholder": "生成中…", "askPlaceholder": "描述你想创建的技能…", "emptyPreview": "生成一个技能后可在这里预览", @@ -619,6 +618,12 @@ "heroTitle": "描述一个技能，让我帮你构建。", "heroSubtitle": "告诉模型这个技能要做什么。它先草拟出技能包，你来迭代修改，最后保存即可。", "drawerHint": "右侧边栏：技能包预览 + 保存", + "modeLabel": "模式", + "modeAria": "生成模式", + "modeSimple": "简单", + "modeAdvanced": "高级", + "modeSimpleHint": "仅 SKILL.md — 不含脚本、参考资料或素材", + "modeAdvancedHint": "SKILL.md 加上脚本、参考资料和素材", "tabPackage": "技能包", "pin": "钉住", "unpin": "取消钉住", diff --git a/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx b/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx index b1026a4b..1e70658b 100644 --- a/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx +++ b/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx @@ -29,9 +29,11 @@ import { ValidationErrorPanel } from "@/components/skill/ValidationErrorPanel"; import { GenerationChatMessage } from "@/components/skill/GenerationChatMessage"; import { GenerativeEmptyHero } from "@/components/skill/generative/GenerativeEmptyHero"; import { GenerativePackageRailTab } from "@/components/skill/generative/GenerativePackageRailTab"; +import { GenerationModeToggle } from "@/components/skill/generative/GenerationModeToggle"; import { ModelPicker } from "@/components/models/ModelPicker"; import { OverLimitPage } from "@/components/quota/OverLimitPage"; import { QuotaInline } from "@/components/quota/QuotaInline"; +import { useGenerationModeCopy, usePreferredGenerationMode } from "@/hooks/useGenerationMode"; import { useGenerativeDrawer } from "@/hooks/useGenerativeDrawer"; import { useSkillGeneration } from "@/hooks/useSkillGeneration"; import { useCreateSkill } from "@/hooks/useSkills"; @@ -80,10 +82,16 @@ export function CreateSkillGenerativePage() { skillGenSnap!.remaining <= 0; const [pickedModelId, setPickedModelId] = useState(null); + // Package shape for the next turn (#1242). Persisted like the model + // pick; sent with every turn so the user can switch between + // refinements (e.g. "now add a script" → advanced). + const [mode, setMode] = usePreferredGenerationMode(); + const modeCopy = useGenerationModeCopy(); const handleSend = useCallback( - (content: string) => generation.sendMessage(content, { modelId: pickedModelId ?? undefined }), - [generation, pickedModelId], + (content: string) => + generation.sendMessage(content, { modelId: pickedModelId ?? undefined, mode }), + [generation, pickedModelId, mode], ); const handleStarterClick = useCallback((body: string) => { @@ -240,10 +248,12 @@ export function CreateSkillGenerativePage() { )}
- {/* Composer — model picker + quota above, ChatGPT-style. */} + {/* Composer — quota + mode + model picker above, ChatGPT-style. + `flex-wrap` lets the three chips restack on narrow viewports. */}
-
+
+
-

+ {/* Always-visible description of the selected mode — hover + `title` on the segments is not a sufficient affordance. */} +

+ {modeCopy.labels[mode]} + {" · "} + {modeCopy.hints[mode]} +

+

{t("playground.kbHint", "Enter to send · Shift + Enter for newline")}

From f1029fb78058db0a1fbf3eb717200776bbb538df Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:46:34 +0800 Subject: [PATCH 14/24] docs(skills): document generation mode in the agent manuals (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three agent manuals (`ornn-agent-manual-cli`, `ornn-agent-manual-http`, `chrono-ai-service-manual`) carry byte-identical §7 text, so the same edit is applied to all three: - §7.0 now spells out the parsed shape of `generation_complete.raw` — including the new `references[]` / `assets[]` arrays — instead of "", and states the simple-mode guarantee (plain category, all six file/runtime arrays empty). - §7.1 gains a field table for `mode` and the previously undocumented `modelId`, shows both in the JSON, multipart and multi-turn examples, explains the retry matrix per body shape, and adds `INVALID_MODE` to the endpoint table and the §1.8 legend. - §7.2 / §7.3 state that from-source / from-openapi are always the equivalent of `simple` and ignore `mode`. - SKILL.md build-flow step 1 in each manual mentions the two modes; each touched SKILL.md bumps `version` / `lastUpdated` so agents that compare against the registry copy (§0) notice the update. Publishing the bumped manuals to the registry is a separate operational step. `skills/ornn-agent-manual-http/SKILL.md` is an assistant KB source, so the digest is regenerated (`bun run build:assistant-kb`) to keep the `assistant-kb-freshness` CI gate green. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/assistant/kb/digest.generated.md | 20 ++----- skills/chrono-ai-service-manual/SKILL.md | 6 +-- .../references/ornn-api-reference.md | 53 ++++++++++++++++--- skills/ornn-agent-manual-cli/SKILL.md | 6 +-- .../references/api-reference.md | 53 ++++++++++++++++--- skills/ornn-agent-manual-http/SKILL.md | 6 +-- .../references/api-reference.md | 53 ++++++++++++++++--- 7 files changed, 149 insertions(+), 48 deletions(-) diff --git a/ornn-api/src/domains/assistant/kb/digest.generated.md b/ornn-api/src/domains/assistant/kb/digest.generated.md index 34dc6225..2ac6525d 100644 --- a/ornn-api/src/domains/assistant/kb/digest.generated.md +++ b/ornn-api/src/domains/assistant/kb/digest.generated.md @@ -3,12 +3,12 @@ Produced by ornn-api/scripts/build-assistant-kb.ts (#970). Re-run: `bun run scripts/build-assistant-kb.ts` from ornn-api/. budgetTokens: 18000 - estimatedTokens: 11360 + estimatedTokens: 11364 sources: - readme: ~2250 tok - claude-positioning: ~272 tok - architecture: ~219 tok - - agent-manual-http: ~5486 tok (clipped) + - agent-manual-http: ~5491 tok (clipped) - conventions: ~2599 tok (clipped) - design-overview: ~487 tok --> @@ -255,8 +255,8 @@ metadata: - manual - skill-lifecycle - http -version: "1.1" -lastUpdated: 2026-04-29 +version: "1.2" +lastUpdated: 2026-09-16 --- # Agent Manual (HTTPS variant) @@ -507,7 +507,7 @@ The response is `{ data: { name, description, metadata, files: { "SKILL.md": ".. **Step 5 — If steps 2–3 yielded nothing after 5 search attempts**, you may decide your own way to perform the task. **And if the task is definitive and potentially repeatable, build a skill and upload it back to Ornn so future you (or other agents) can find it.** Build flow: -1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). Useful when you need a starter; the generated skill still needs validation + your edits. +1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). On the prompt endpoint pass `"mode": "simple"` for a single `SKILL.md` (server-enforced — no scripts / references / assets) or leave the default `"advanced"` to let the model add `scripts/`, `references/` and `assets/`. Useful when you need a starter; the generated skill still needs validation + your edits. 2. **Read the skill format spec** so you write a valid one: @@ -600,16 +600,6 @@ curl -X PUT \ -H "Authorization: Bearer $TOKEN" \ -H "Content-Type: application/json" \ -d '{"isPrivate":true,"sharedWithUsers":["user_abc"],"sharedWithOrgs":["org_xyz"]}' \ - "https://ornn.chrono-ai.fun/api/v1/skills//permissions" -``` - -**Step 3c — Set to private.** - -```bash -curl -X PUT \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d --- diff --git a/skills/chrono-ai-service-manual/SKILL.md b/skills/chrono-ai-service-manual/SKILL.md index 296d2ba8..cbe0ae12 100644 --- a/skills/chrono-ai-service-manual/SKILL.md +++ b/skills/chrono-ai-service-manual/SKILL.md @@ -11,8 +11,8 @@ metadata: - manual - identity - skill-lifecycle -version: "1.1" -lastUpdated: 2026-06-11 +version: "1.2" +lastUpdated: 2026-09-16 --- # Chrono AI Service Manual @@ -573,7 +573,7 @@ The response is `{ data: { name, description, metadata, files: { "SKILL.md": ".. **Step 5 — If steps 2–3 yielded nothing after 5 search attempts**, you may decide your own way to perform the task. **And if the task is definitive and potentially repeatable, build a skill and upload it back to Ornn.** Build flow: -1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). The generated skill still needs validation + your edits. +1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). On the prompt endpoint pass `"mode": "simple"` for a single `SKILL.md` (server-enforced — no scripts / references / assets) or leave the default `"advanced"` to let the model add `scripts/`, `references/` and `assets/`. The generated skill still needs validation + your edits. 2. **Read the skill format spec** so you write a valid one: diff --git a/skills/chrono-ai-service-manual/references/ornn-api-reference.md b/skills/chrono-ai-service-manual/references/ornn-api-reference.md index 44f00c79..1877c5fa 100644 --- a/skills/chrono-ai-service-manual/references/ornn-api-reference.md +++ b/skills/chrono-ai-service-manual/references/ornn-api-reference.md @@ -170,6 +170,7 @@ The codes below appear across many endpoints. Per-endpoint sections list any add | `INVALID_DEPRECATION_PATCH` | 400 | Body for `PATCH /versions/:version` is malformed. | | `INVALID_PERMISSIONS` | 400 | Body for `PUT /skills/:id/permissions` is malformed. | | `MISSING_PROMPT` / `MISSING_REPO` / `MISSING_SOURCE` / `MISSING_SPEC` | 400 | Required JSON field absent on the relevant generation / pull endpoint. | +| `INVALID_MODE` | 400 | `POST /skills/generate` `mode` is not `simple` or `advanced` (§7.1). | | `AMBIGUOUS_SOURCE` | 400 | `/skills/generate/from-source` got both `code` and `repoUrl`. | | `EMPTY_SOURCE` | 400 | `/skills/generate/from-source` got an empty `code` after fetching. | | `REPO_FETCH_FAILED` | 400 | `/skills/generate/from-source` could not fetch the requested GitHub repo. | @@ -1253,7 +1254,7 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "token", "content": "partial output text" } // generation_complete (terminal — full result) -{ "type": "generation_complete", "raw": "" } +{ "type": "generation_complete", "raw": "" } // validation_error (auto-retry; pipeline keeps streaming) { "type": "validation_error", "message": "...", "retrying": true } @@ -1265,11 +1266,37 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "keepalive" } ``` -A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. The `raw` field is a serialised representation of the generated skill; clients should parse it back into the package structure (frontmatter + files). +A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. + +`raw` is a JSON **document string** (not a ZIP, not markdown). Parse it to get: + +```jsonc +{ + "name": "kebab-case-name", + "description": "...", + "category": "plain" | "runtime-based", + "outputType": "text" | "file", // runtime-based only + "tags": ["..."], + "readmeBody": "", + "runtimes": ["node"] | ["python"] | [], + "dependencies": ["..."], + "envVars": ["..."], + "scripts": [{ "filename": "main.js", "content": "..." }], // → scripts/ + "references": [{ "filename": "notes.md", "content": "..." }], // → references/ + "assets": [{ "filename": "data.csv", "content": "..." }] // → assets/ (text only) +} +``` + +The three file arrays are always present (empty when unused). Nothing is persisted — assemble the package yourself and publish it with `POST /api/v1/skills` (§2.2). **In `simple` mode (§7.1) the server guarantees `category` is `plain` and all six of `scripts` / `references` / `assets` / `runtimes` / `dependencies` / `envVars` are empty** — an answer that violates that never reaches `generation_complete`. ### 7.1 Generate from prompt — `POST /api/v1/skills/generate` -Generate a fresh skill from a natural-language prompt. Two body modes: single-shot and multi-turn. +Generate a fresh skill from a natural-language prompt. Two body shapes: single-shot and multi-turn. Both accept the same two optional fields: + +| Field | Values | Default | Meaning | +|---|---|---|---| +| `mode` | `"simple"` \| `"advanced"` | `"advanced"` | Package shape. `advanced` = the model may emit `scripts[]`, `references[]`, `assets[]` (and pick `runtime-based`). `simple` = one `SKILL.md`, nothing else — the model is told to keep everything inline and the server **rejects** any answer that carries files or a non-plain category (one corrective retry, then `error`). Anything else → 400 `INVALID_MODE` before the quota reserve. | +| `modelId` | id from `GET /api/v1/me/models?surface=skillGen` (§11) | surface default | Admin-curated model to use. | **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1277,16 +1304,22 @@ Generate a fresh skill from a natural-language prompt. Two body modes: single-sh ```jsonc // JSON -{ "prompt": "Build a skill that converts CSV to JSON using csv-parse" } +{ + "prompt": "Build a skill that converts CSV to JSON using csv-parse", + "mode": "simple", // optional — omit for "advanced" + "modelId": "gpt-4.1-mini" // optional +} ``` ```text # multipart/form-data prompt=Build a skill that converts CSV to JSON ... +mode=simple # optional — plain form field, same semantics as JSON +modelId=gpt-4.1-mini # optional package=@existing-skill.zip # optional — iterate on an existing package ``` -When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content". +When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content" — even in `simple` mode, where the existing `scripts/` are read as context but the answer must still be `SKILL.md`-only. #### 7.1.b Multi-turn (JSON only) @@ -1296,24 +1329,28 @@ When `package` is included, its file contents are extracted (SKILL.md + scripts/ { "role": "user", "content": "Build a CSV-to-JSON skill" }, { "role": "assistant", "content": "" }, { "role": "user", "content": "Switch to csv-parse from papaparse" } - ] + ], + "mode": "advanced" // optional — applies to THIS turn; you may switch between turns } ``` Same SSE event types, but the model has the prior turns as conversational context. +Retry behaviour differs between the two shapes. Single-shot re-asks the model once for **any** rejected first answer (not JSON, schema-invalid, or a `simple`-mode violation) — you see `validation_error` with `retrying: true` and the run may end on `error` with no `generation_complete`. Multi-turn does **not** retry a merely non-JSON answer (a refinement turn may legitimately be prose): it emits `validation_error` with `retrying: false` and still delivers that text in `generation_complete`, so re-validate `raw` before trusting it. The one multi-turn exception is a `simple`-mode violation, which gets the same single corrective retry and ends on `error` if the model still emits files. + Response: SSE stream as in §7.0. No JSON envelope. | Code (in stream) | Cause | |---|---| | `MISSING_PROMPT` | Neither `prompt` nor `messages` present. | +| `INVALID_MODE` | `mode` is not `simple` or `advanced` (400 before the stream and before any quota reserve). | | `INVALID_CONTENT_TYPE` | Wrong Content-Type — must be `application/json` or `multipart/form-data`. | | `AUTH_MISSING` | 401 returned **before** the stream starts. | | `FORBIDDEN` | 403 returned before the stream starts (missing `ornn:skill:build`). | ### 7.2 Generate from source — `POST /api/v1/skills/generate/from-source` -Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. +Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. Always produces the equivalent of a `simple` package (a `plain` skill with empty `scripts` / `references` / `assets`); there is no `mode` field here — one sent in the body is ignored, not rejected. **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1341,7 +1378,7 @@ Response: SSE stream (§7.0). ### 7.3 Generate from OpenAPI — `POST /api/v1/skills/generate/from-openapi` -Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. +Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. Like §7.2 this always produces the equivalent of a `simple` package and takes no `mode` field (ignored if sent). **Auth: required.** **Permission: `ornn:skill:build`.** diff --git a/skills/ornn-agent-manual-cli/SKILL.md b/skills/ornn-agent-manual-cli/SKILL.md index da2d00fd..2cca55f8 100644 --- a/skills/ornn-agent-manual-cli/SKILL.md +++ b/skills/ornn-agent-manual-cli/SKILL.md @@ -9,8 +9,8 @@ metadata: - manual - skill-lifecycle - cli -version: "1.5" -lastUpdated: 2026-07-01 +version: "1.6" +lastUpdated: 2026-09-16 --- # Agent Manual (NyxID CLI variant) @@ -241,7 +241,7 @@ The response is `{ data: { name, description, metadata, files: { "SKILL.md": ".. **Step 5 — If steps 2–3 yielded nothing after 5 search attempts**, you may decide your own way to perform the task. **And if the task is definitive and potentially repeatable, build a skill and upload it back to Ornn so future you (or other agents) can find it.** Build flow: -1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). Useful when you need a starter; the generated skill still needs validation + your edits. +1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). On the prompt endpoint pass `"mode": "simple"` for a single `SKILL.md` (server-enforced — no scripts / references / assets) or leave the default `"advanced"` to let the model add `scripts/`, `references/` and `assets/`. Useful when you need a starter; the generated skill still needs validation + your edits. 2. **Read the skill format spec** so you write a valid one: diff --git a/skills/ornn-agent-manual-cli/references/api-reference.md b/skills/ornn-agent-manual-cli/references/api-reference.md index 39d30f43..baf8214e 100644 --- a/skills/ornn-agent-manual-cli/references/api-reference.md +++ b/skills/ornn-agent-manual-cli/references/api-reference.md @@ -173,6 +173,7 @@ The codes below appear across many endpoints. Per-endpoint sections list any add | `INVALID_DEPRECATION_PATCH` | 400 | Body for `PATCH /versions/:version` is malformed. | | `INVALID_PERMISSIONS` | 400 | Body for `PUT /skills/:id/permissions` is malformed. | | `MISSING_PROMPT` / `MISSING_REPO` / `MISSING_SOURCE` / `MISSING_SPEC` | 400 | Required JSON field absent on the relevant generation / pull endpoint. | +| `INVALID_MODE` | 400 | `POST /skills/generate` `mode` is not `simple` or `advanced` (§7.1). | | `AMBIGUOUS_SOURCE` | 400 | `/skills/generate/from-source` got both `code` and `repoUrl`. | | `EMPTY_SOURCE` | 400 | `/skills/generate/from-source` got an empty `code` after fetching. | | `REPO_FETCH_FAILED` | 400 | `/skills/generate/from-source` could not fetch the requested GitHub repo. | @@ -1465,7 +1466,7 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "token", "content": "partial output text" } // generation_complete (terminal — full result) -{ "type": "generation_complete", "raw": "" } +{ "type": "generation_complete", "raw": "" } // validation_error (auto-retry; pipeline keeps streaming) { "type": "validation_error", "message": "...", "retrying": true } @@ -1477,11 +1478,37 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "keepalive" } ``` -A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. The `raw` field is a serialised representation of the generated skill; clients should parse it back into the package structure (frontmatter + files). +A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. + +`raw` is a JSON **document string** (not a ZIP, not markdown). Parse it to get: + +```jsonc +{ + "name": "kebab-case-name", + "description": "...", + "category": "plain" | "runtime-based", + "outputType": "text" | "file", // runtime-based only + "tags": ["..."], + "readmeBody": "", + "runtimes": ["node"] | ["python"] | [], + "dependencies": ["..."], + "envVars": ["..."], + "scripts": [{ "filename": "main.js", "content": "..." }], // → scripts/ + "references": [{ "filename": "notes.md", "content": "..." }], // → references/ + "assets": [{ "filename": "data.csv", "content": "..." }] // → assets/ (text only) +} +``` + +The three file arrays are always present (empty when unused). Nothing is persisted — assemble the package yourself and publish it with `POST /api/v1/skills` (§2.2). **In `simple` mode (§7.1) the server guarantees `category` is `plain` and all six of `scripts` / `references` / `assets` / `runtimes` / `dependencies` / `envVars` are empty** — an answer that violates that never reaches `generation_complete`. ### 7.1 Generate from prompt — `POST /api/v1/skills/generate` -Generate a fresh skill from a natural-language prompt. Two body modes: single-shot and multi-turn. +Generate a fresh skill from a natural-language prompt. Two body shapes: single-shot and multi-turn. Both accept the same two optional fields: + +| Field | Values | Default | Meaning | +|---|---|---|---| +| `mode` | `"simple"` \| `"advanced"` | `"advanced"` | Package shape. `advanced` = the model may emit `scripts[]`, `references[]`, `assets[]` (and pick `runtime-based`). `simple` = one `SKILL.md`, nothing else — the model is told to keep everything inline and the server **rejects** any answer that carries files or a non-plain category (one corrective retry, then `error`). Anything else → 400 `INVALID_MODE` before the quota reserve. | +| `modelId` | id from `GET /api/v1/me/models?surface=skillGen` (§11) | surface default | Admin-curated model to use. | **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1489,16 +1516,22 @@ Generate a fresh skill from a natural-language prompt. Two body modes: single-sh ```jsonc // JSON -{ "prompt": "Build a skill that converts CSV to JSON using csv-parse" } +{ + "prompt": "Build a skill that converts CSV to JSON using csv-parse", + "mode": "simple", // optional — omit for "advanced" + "modelId": "gpt-4.1-mini" // optional +} ``` ```text # multipart/form-data prompt=Build a skill that converts CSV to JSON ... +mode=simple # optional — plain form field, same semantics as JSON +modelId=gpt-4.1-mini # optional package=@existing-skill.zip # optional — iterate on an existing package ``` -When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content". +When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content" — even in `simple` mode, where the existing `scripts/` are read as context but the answer must still be `SKILL.md`-only. #### 7.1.b Multi-turn (JSON only) @@ -1508,24 +1541,28 @@ When `package` is included, its file contents are extracted (SKILL.md + scripts/ { "role": "user", "content": "Build a CSV-to-JSON skill" }, { "role": "assistant", "content": "" }, { "role": "user", "content": "Switch to csv-parse from papaparse" } - ] + ], + "mode": "advanced" // optional — applies to THIS turn; you may switch between turns } ``` Same SSE event types, but the model has the prior turns as conversational context. +Retry behaviour differs between the two shapes. Single-shot re-asks the model once for **any** rejected first answer (not JSON, schema-invalid, or a `simple`-mode violation) — you see `validation_error` with `retrying: true` and the run may end on `error` with no `generation_complete`. Multi-turn does **not** retry a merely non-JSON answer (a refinement turn may legitimately be prose): it emits `validation_error` with `retrying: false` and still delivers that text in `generation_complete`, so re-validate `raw` before trusting it. The one multi-turn exception is a `simple`-mode violation, which gets the same single corrective retry and ends on `error` if the model still emits files. + Response: SSE stream as in §7.0. No JSON envelope. | Code (in stream) | Cause | |---|---| | `MISSING_PROMPT` | Neither `prompt` nor `messages` present. | +| `INVALID_MODE` | `mode` is not `simple` or `advanced` (400 before the stream and before any quota reserve). | | `INVALID_CONTENT_TYPE` | Wrong Content-Type — must be `application/json` or `multipart/form-data`. | | `AUTH_MISSING` | 401 returned **before** the stream starts. | | `FORBIDDEN` | 403 returned before the stream starts (missing `ornn:skill:build`). | ### 7.2 Generate from source — `POST /api/v1/skills/generate/from-source` -Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. +Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. Always produces the equivalent of a `simple` package (a `plain` skill with empty `scripts` / `references` / `assets`); there is no `mode` field here — one sent in the body is ignored, not rejected. **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1553,7 +1590,7 @@ Response: SSE stream (§7.0). ### 7.3 Generate from OpenAPI — `POST /api/v1/skills/generate/from-openapi` -Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. +Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. Like §7.2 this always produces the equivalent of a `simple` package and takes no `mode` field (ignored if sent). **Auth: required.** **Permission: `ornn:skill:build`.** diff --git a/skills/ornn-agent-manual-http/SKILL.md b/skills/ornn-agent-manual-http/SKILL.md index e8a177d0..a21abf47 100644 --- a/skills/ornn-agent-manual-http/SKILL.md +++ b/skills/ornn-agent-manual-http/SKILL.md @@ -9,8 +9,8 @@ metadata: - manual - skill-lifecycle - http -version: "1.1" -lastUpdated: 2026-04-29 +version: "1.2" +lastUpdated: 2026-09-16 --- # Agent Manual (HTTPS variant) @@ -261,7 +261,7 @@ The response is `{ data: { name, description, metadata, files: { "SKILL.md": ".. **Step 5 — If steps 2–3 yielded nothing after 5 search attempts**, you may decide your own way to perform the task. **And if the task is definitive and potentially repeatable, build a skill and upload it back to Ornn so future you (or other agents) can find it.** Build flow: -1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). Useful when you need a starter; the generated skill still needs validation + your edits. +1. *(Optional)* **Bootstrap with AI generation** — Ornn's LLM can scaffold a skill from a prompt, source code, or an OpenAPI spec via `POST /api/v1/skills/generate*` (SSE). On the prompt endpoint pass `"mode": "simple"` for a single `SKILL.md` (server-enforced — no scripts / references / assets) or leave the default `"advanced"` to let the model add `scripts/`, `references/` and `assets/`. Useful when you need a starter; the generated skill still needs validation + your edits. 2. **Read the skill format spec** so you write a valid one: diff --git a/skills/ornn-agent-manual-http/references/api-reference.md b/skills/ornn-agent-manual-http/references/api-reference.md index bd32c4d4..e7009f1c 100644 --- a/skills/ornn-agent-manual-http/references/api-reference.md +++ b/skills/ornn-agent-manual-http/references/api-reference.md @@ -171,6 +171,7 @@ The codes below appear across many endpoints. Per-endpoint sections list any add | `INVALID_DEPRECATION_PATCH` | 400 | Body for `PATCH /versions/:version` is malformed. | | `INVALID_PERMISSIONS` | 400 | Body for `PUT /skills/:id/permissions` is malformed. | | `MISSING_PROMPT` / `MISSING_REPO` / `MISSING_SOURCE` / `MISSING_SPEC` | 400 | Required JSON field absent on the relevant generation / pull endpoint. | +| `INVALID_MODE` | 400 | `POST /skills/generate` `mode` is not `simple` or `advanced` (§7.1). | | `AMBIGUOUS_SOURCE` | 400 | `/skills/generate/from-source` got both `code` and `repoUrl`. | | `EMPTY_SOURCE` | 400 | `/skills/generate/from-source` got an empty `code` after fetching. | | `REPO_FETCH_FAILED` | 400 | `/skills/generate/from-source` could not fetch the requested GitHub repo. | @@ -1347,7 +1348,7 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "token", "content": "partial output text" } // generation_complete (terminal — full result) -{ "type": "generation_complete", "raw": "" } +{ "type": "generation_complete", "raw": "" } // validation_error (auto-retry; pipeline keeps streaming) { "type": "validation_error", "message": "...", "retrying": true } @@ -1359,11 +1360,37 @@ Three endpoints, all SSE. All require `ornn:skill:build`. All emit the same even { "type": "keepalive" } ``` -A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. The `raw` field is a serialised representation of the generated skill; clients should parse it back into the package structure (frontmatter + files). +A normal stream ends with `generation_complete` followed by the proxy closing the connection. A fatal stream ends with `error`. + +`raw` is a JSON **document string** (not a ZIP, not markdown). Parse it to get: + +```jsonc +{ + "name": "kebab-case-name", + "description": "...", + "category": "plain" | "runtime-based", + "outputType": "text" | "file", // runtime-based only + "tags": ["..."], + "readmeBody": "", + "runtimes": ["node"] | ["python"] | [], + "dependencies": ["..."], + "envVars": ["..."], + "scripts": [{ "filename": "main.js", "content": "..." }], // → scripts/ + "references": [{ "filename": "notes.md", "content": "..." }], // → references/ + "assets": [{ "filename": "data.csv", "content": "..." }] // → assets/ (text only) +} +``` + +The three file arrays are always present (empty when unused). Nothing is persisted — assemble the package yourself and publish it with `POST /api/v1/skills` (§2.2). **In `simple` mode (§7.1) the server guarantees `category` is `plain` and all six of `scripts` / `references` / `assets` / `runtimes` / `dependencies` / `envVars` are empty** — an answer that violates that never reaches `generation_complete`. ### 7.1 Generate from prompt — `POST /api/v1/skills/generate` -Generate a fresh skill from a natural-language prompt. Two body modes: single-shot and multi-turn. +Generate a fresh skill from a natural-language prompt. Two body shapes: single-shot and multi-turn. Both accept the same two optional fields: + +| Field | Values | Default | Meaning | +|---|---|---|---| +| `mode` | `"simple"` \| `"advanced"` | `"advanced"` | Package shape. `advanced` = the model may emit `scripts[]`, `references[]`, `assets[]` (and pick `runtime-based`). `simple` = one `SKILL.md`, nothing else — the model is told to keep everything inline and the server **rejects** any answer that carries files or a non-plain category (one corrective retry, then `error`). Anything else → 400 `INVALID_MODE` before the quota reserve. | +| `modelId` | id from `GET /api/v1/me/models?surface=skillGen` (§11) | surface default | Admin-curated model to use. | **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1371,16 +1398,22 @@ Generate a fresh skill from a natural-language prompt. Two body modes: single-sh ```jsonc // JSON -{ "prompt": "Build a skill that converts CSV to JSON using csv-parse" } +{ + "prompt": "Build a skill that converts CSV to JSON using csv-parse", + "mode": "simple", // optional — omit for "advanced" + "modelId": "gpt-4.1-mini" // optional +} ``` ```text # multipart/form-data prompt=Build a skill that converts CSV to JSON ... +mode=simple # optional — plain form field, same semantics as JSON +modelId=gpt-4.1-mini # optional package=@existing-skill.zip # optional — iterate on an existing package ``` -When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content". +When `package` is included, its file contents are extracted (SKILL.md + scripts/ + references/ + assets/) and included in the prompt as "existing skill content" — even in `simple` mode, where the existing `scripts/` are read as context but the answer must still be `SKILL.md`-only. #### 7.1.b Multi-turn (JSON only) @@ -1390,24 +1423,28 @@ When `package` is included, its file contents are extracted (SKILL.md + scripts/ { "role": "user", "content": "Build a CSV-to-JSON skill" }, { "role": "assistant", "content": "" }, { "role": "user", "content": "Switch to csv-parse from papaparse" } - ] + ], + "mode": "advanced" // optional — applies to THIS turn; you may switch between turns } ``` Same SSE event types, but the model has the prior turns as conversational context. +Retry behaviour differs between the two shapes. Single-shot re-asks the model once for **any** rejected first answer (not JSON, schema-invalid, or a `simple`-mode violation) — you see `validation_error` with `retrying: true` and the run may end on `error` with no `generation_complete`. Multi-turn does **not** retry a merely non-JSON answer (a refinement turn may legitimately be prose): it emits `validation_error` with `retrying: false` and still delivers that text in `generation_complete`, so re-validate `raw` before trusting it. The one multi-turn exception is a `simple`-mode violation, which gets the same single corrective retry and ends on `error` if the model still emits files. + Response: SSE stream as in §7.0. No JSON envelope. | Code (in stream) | Cause | |---|---| | `MISSING_PROMPT` | Neither `prompt` nor `messages` present. | +| `INVALID_MODE` | `mode` is not `simple` or `advanced` (400 before the stream and before any quota reserve). | | `INVALID_CONTENT_TYPE` | Wrong Content-Type — must be `application/json` or `multipart/form-data`. | | `AUTH_MISSING` | 401 returned **before** the stream starts. | | `FORBIDDEN` | 403 returned before the stream starts (missing `ornn:skill:build`). | ### 7.2 Generate from source — `POST /api/v1/skills/generate/from-source` -Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. +Generate a skill by analysing existing source code — either an inline snippet or a public GitHub repo. Always produces the equivalent of a `simple` package (a `plain` skill with empty `scripts` / `references` / `assets`); there is no `mode` field here — one sent in the body is ignored, not rejected. **Auth: required.** **Permission: `ornn:skill:build`.** @@ -1435,7 +1472,7 @@ Response: SSE stream (§7.0). ### 7.3 Generate from OpenAPI — `POST /api/v1/skills/generate/from-openapi` -Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. +Generate a skill that wraps one or more endpoints from an OpenAPI 3 spec. Like §7.2 this always produces the equivalent of a `simple` package and takes no `mode` field (ignored if sent). **Auth: required.** **Permission: `ornn:skill:build`.** From 0302e3c14a8d49b35fc72dc41551173e6e99771e Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:46:49 +0800 Subject: [PATCH 15/24] docs: changeset for skill generation modes (#1242) Minor bump for both packages: a new optional request field on `POST /skills/generate` plus new arrays in the generation output contract (api), and the mode toggle + references/assets preview (web). Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .changeset/skill-generation-modes.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/skill-generation-modes.md diff --git a/.changeset/skill-generation-modes.md b/.changeset/skill-generation-modes.md new file mode 100644 index 00000000..155a5d40 --- /dev/null +++ b/.changeset/skill-generation-modes.md @@ -0,0 +1,6 @@ +--- +"ornn-api": minor +"ornn-web": minor +--- + +Add caller-chosen, server-enforced generation modes to `POST /api/v1/skills/generate` (#1242). `mode: "simple"` asks for a single `SKILL.md`: the model gets a dedicated prompt and the server rejects any answer that is not a plain, file-less skill (one corrective retry, then a terminal `error`), so `generation_complete.raw` in simple mode never carries scripts, references or assets. `mode: "advanced"` — the default, and the pre-existing behaviour — lets the model emit `scripts[]` and, newly, `references[]` and `assets[]` text files. An unknown value fails with 400 `invalid_mode` before the quota reserve. The generative skill builder in ornn-web gains a Simple | Advanced toggle in the composer row (persisted like the model pick, locked while streaming), builds `references/` and `assets/` folders in the package preview, and now surfaces the server's problem+json detail when a generation request is rejected before the stream opens. The OpenAPI spec and the three agent manuals document the new field, the extended `raw` shape and the simple-mode guarantee. From c5267d67ba864b9ab05aa5d58c0fcec74d52d399 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:50:31 +0800 Subject: [PATCH 16/24] test(api): integration coverage for generation mode end to end (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drives `mode` through the real Hono app, settings-driven model resolution, quota buckets and the generation service with only the LLM client injected (the harness seam the charge tests already use): - `mode: "bogus"` → 400 `invalid_mode` problem+json, no bucket row. - `simple` + scripted first answer → `validation_error` (retrying) → the `complete()` retry carries the simple prompt and the corrective instruction → `generation_complete` with the plain answer → charged once. - `simple` + scripted answer twice (multi-turn) → terminal `error`, no `generation_complete`, the offending answer replayed as an assistant turn on the retry, still charged once (skill_error). - multipart `mode=simple` reaches the service. - omitted mode (advanced) passes references / assets straight through. The gateway mock reuses one `text` for stream and retry, so the test composes its stream with a `complete()` that answers differently — the simple-mode contract is exactly "first answer bad, retry good". Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../tests/integration/skillgen_mode.test.ts | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 ornn-api/tests/integration/skillgen_mode.test.ts diff --git a/ornn-api/tests/integration/skillgen_mode.test.ts b/ornn-api/tests/integration/skillgen_mode.test.ts new file mode 100644 index 00000000..8df6b10f --- /dev/null +++ b/ornn-api/tests/integration/skillgen_mode.test.ts @@ -0,0 +1,332 @@ +/** + * IT-SKILLGEN-MODE-* — end-to-end contract of the `mode` field on + * `POST /api/v1/skills/generate` (#1242) through the real Hono app, + * settings-driven model resolution, quota buckets and the generation + * service, with only the LLM client injected. + * + * - `mode: "simple"`, model answers with files → `validation_error` + * (retrying) → corrective retry via `complete()` carries the + * simple-mode instruction → `generation_complete` with the plain + * answer → charged once. + * - `mode: "simple"`, model answers with files twice → terminal + * `error`, no `generation_complete` → still charged once + * (skill_error, same as the invalid-JSON retry rule). + * - `mode` omitted → advanced: a scripted answer with references / + * assets flows straight to `generation_complete`. + * - `mode: "bogus"` → 400 `invalid_mode` problem+json, LLM never + * called, no bucket row. + * - multipart `mode=simple` form field reaches the service. + * + * @module tests/integration/skillgen_mode.test + */ + +import { afterAll, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { startHarness, type Harness, authHeaders } from "./harness"; +import { resetCollections } from "./cleanup"; +import { installLlmGatewayMock } from "../mocks/llmGateway"; +import type { + NyxLlmClient, + NyxLlmCompleteParams, + ResponsesApiOutput, +} from "../../src/clients/nyxid/llm"; +import { + SIMPLE_GENERATION_SYSTEM_PROMPT, + SIMPLE_MODE_RETRY_INSTRUCTION, +} from "../../src/domains/skills/generation/prompts"; + +const buildAuth = (userId: string) => + authHeaders({ + userId, + email: `${userId}@test.invalid`, + permissions: ["ornn:skill:build"], + }); + +/** Plain, SKILL.md-only answer — legal in either mode. */ +const PLAIN_JSON = JSON.stringify({ + name: "plain-skill", + description: "A plain test skill generated for the mode integration test.", + category: "plain", + tags: ["test", "integration"], + readmeBody: + "This is a generated test skill body long enough to clear the fifty character minimum readme length requirement.", +}); + +/** Schema-valid answer that carries files — legal in advanced, a violation in simple. */ +const SCRIPTED_JSON = JSON.stringify({ + ...JSON.parse(PLAIN_JSON), + name: "scripted-skill", + category: "runtime-based", + outputType: "text", + runtimes: ["node"], + dependencies: ["axios"], + envVars: ["API_KEY"], + scripts: [{ filename: "main.js", content: "console.log('hi')" }], + references: [{ filename: "notes.md", content: "# Notes" }], + assets: [{ filename: "sample.csv", content: "a,b\n1,2" }], +}); + +/** Seed one provider with a model enabled for the skillGen surface. */ +async function seedSkillGenModel(db: Harness["db"], modelId: string): Promise { + const now = new Date(); + await db.collection("llm_providers").insertOne({ + _id: "prov-skillgen", + name: "test-provider", + gatewayUrl: "https://gw.test.invalid", + modelListUrl: "https://gw.test.invalid/models", + apiFormat: "responses", + auth: { kind: "apiKey", apiKeyEnc: "" }, + models: [ + { + id: modelId, + displayName: modelId, + enabledForPlayground: false, + enabledForSkillGen: true, + defaultForPlayground: false, + defaultForSkillGen: true, + removed: false, + firstSeenAt: now, + lastSyncedAt: now, + }, + ], + maxOutputTokens: 8192, + defaultTemperature: 0.7, + createdAt: now, + updatedAt: now, + updatedBy: "test", + }); +} + +const monthMarker = () => new Date().toISOString().slice(0, 7); + +/** Read the whole SSE body and return the parsed `data:` payloads in order. */ +async function readFrames(res: Response): Promise>> { + const text = await res.text(); + const frames: Array> = []; + for (const block of text.split("\n\n")) { + for (const line of block.split("\n")) { + if (!line.startsWith("data:")) continue; + const payload = line.slice(5).trim(); + if (!payload) continue; + frames.push(JSON.parse(payload) as Record); + } + } + return frames; +} + +async function waitForBucket( + db: Harness["db"], + id: string, + predicate: (doc: Record | null) => boolean, + { tries = 100, intervalMs = 10 } = {}, +): Promise | null> { + for (let i = 0; i < tries; i++) { + const doc = (await db.collection("quota_buckets").findOne({ _id: id } as never)) as + | Record + | null; + if (predicate(doc)) return doc; + await new Promise((r) => setTimeout(r, intervalMs)); + } + return (await db.collection("quota_buckets").findOne({ _id: id } as never)) as + | Record + | null; +} + +/** + * LLM double whose streamed first answer and non-streaming retry answer + * differ — the gateway mock reuses one `text` for both, but the + * simple-mode retry contract is exactly "first answer bad, retry good". + */ +function makeClient(streamText: string, retryText: string): { + client: NyxLlmClient; + completeCalls: NyxLlmCompleteParams[]; + streamCount: () => number; +} { + const { client, handle } = installLlmGatewayMock({ + outcome: "success", + modelId: "gpt-test", + text: streamText, + }); + const completeCalls: NyxLlmCompleteParams[] = []; + const patched = { + stream: (client as { stream: NyxLlmClient["stream"] }).stream, + async complete(params: NyxLlmCompleteParams): Promise { + completeCalls.push(params); + return [{ type: "message", content: [{ type: "output_text", text: retryText }] }]; + }, + }; + return { + client: patched as unknown as NyxLlmClient, + completeCalls, + streamCount: () => handle.callCount(), + }; +} + +let h: Harness; + +beforeAll(async () => { + h = await startHarness(); +}); + +afterAll(async () => { + await h.cleanup(); +}, 30_000); + +beforeEach(async () => { + await resetCollections(h.db, ["quota_buckets", "platform_settings", "llm_providers"]); +}); + +describe("IT-SKILLGEN-MODE-REJECT", () => { + test("unknown mode → 400 invalid_mode before any LLM call or quota reserve", async () => { + await seedSkillGenModel(h.db, "gpt-test"); + const res = await h.app.request("/api/v1/skills/generate", { + method: "POST", + headers: { ...buildAuth("u-badmode"), "Content-Type": "application/json" }, + body: JSON.stringify({ prompt: "hello", mode: "bogus" }), + }); + expect(res.status).toBe(400); + expect(res.headers.get("content-type")).toContain("application/problem+json"); + const body = (await res.json()) as { code: string; detail: string }; + expect(body.code).toBe("invalid_mode"); + expect(body.detail).toContain("simple, advanced"); + const buckets = await h.db + .collection("quota_buckets") + .find({ userId: "u-badmode", surface: "skillGen" }) + .toArray(); + expect(buckets.length).toBe(0); + }); +}); + +describe("IT-SKILLGEN-MODE-SIMPLE (via injected LLM double)", () => { + test("scripted first answer → corrective retry → plain generation_complete, charged once", async () => { + const { client, completeCalls } = makeClient(SCRIPTED_JSON, PLAIN_JSON); + const oh = await startHarness({ llmClient: client }); + try { + await resetCollections(oh.db, ["quota_buckets", "platform_settings", "llm_providers"]); + await seedSkillGenModel(oh.db, "gpt-test"); + + const res = await oh.app.request("/api/v1/skills/generate", { + method: "POST", + headers: { ...buildAuth("u-simple-ok"), "Content-Type": "application/json" }, + body: JSON.stringify({ prompt: "build me a skill", mode: "simple" }), + }); + expect(res.status).toBe(200); + const frames = await readFrames(res); + const types = frames.map((f) => f.type); + expect(types).toEqual(["generation_start", "token", "validation_error", "generation_complete"]); + + const ve = frames[2]!; + expect(ve.retrying).toBe(true); + expect(String(ve.message)).toContain("Simple mode allows SKILL.md only"); + expect(String(ve.message)).toContain("scripts"); + + // The delivered package is the plain retry answer, never the scripted one. + const raw = JSON.parse(String(frames[3]!.raw)) as { name: string; scripts?: unknown[] }; + expect(raw.name).toBe("plain-skill"); + expect(raw.scripts ?? []).toHaveLength(0); + + // Retry carried the simple prompt + the simple-mode instruction. + expect(completeCalls).toHaveLength(1); + const input = completeCalls[0]!.input; + expect(input[0]!.content).toBe(SIMPLE_GENERATION_SYSTEM_PROMPT); + expect(String(input.at(-1)!.content)).toContain(SIMPLE_MODE_RETRY_INSTRUCTION); + + const bucket = await waitForBucket( + oh.db, + `u-simple-ok:skillGen:${monthMarker()}`, + (d) => !!d && (d.used as number) === 1, + ); + expect(bucket?.used).toBe(1); + } finally { + await oh.cleanup(); + } + }); + + test("scripted answer twice → terminal error, no generation_complete, still charged once", async () => { + const { client, completeCalls } = makeClient(SCRIPTED_JSON, SCRIPTED_JSON); + const oh = await startHarness({ llmClient: client }); + try { + await resetCollections(oh.db, ["quota_buckets", "platform_settings", "llm_providers"]); + await seedSkillGenModel(oh.db, "gpt-test"); + + const res = await oh.app.request("/api/v1/skills/generate", { + method: "POST", + headers: { ...buildAuth("u-simple-fail"), "Content-Type": "application/json" }, + body: JSON.stringify({ messages: [{ role: "user", content: "build me a skill" }], mode: "simple" }), + }); + expect(res.status).toBe(200); + const frames = await readFrames(res); + const types = frames.map((f) => f.type); + expect(types).toEqual(["generation_start", "token", "validation_error", "error"]); + expect(String(frames[3]!.message)).toContain("simple mode after retry"); + expect(completeCalls).toHaveLength(1); + // Multi-turn retry continues the conversation with the offending answer. + expect(completeCalls[0]!.input.at(-2)).toEqual({ role: "assistant", content: SCRIPTED_JSON }); + + // validation_error was emitted → skill_error → the slot is consumed. + const bucket = await waitForBucket( + oh.db, + `u-simple-fail:skillGen:${monthMarker()}`, + (d) => !!d && (d.used as number) === 1, + ); + expect(bucket?.used).toBe(1); + } finally { + await oh.cleanup(); + } + }); + + test("multipart mode=simple form field reaches the service", async () => { + const { client, completeCalls, streamCount } = makeClient(PLAIN_JSON, PLAIN_JSON); + const oh = await startHarness({ llmClient: client }); + try { + await resetCollections(oh.db, ["quota_buckets", "platform_settings", "llm_providers"]); + await seedSkillGenModel(oh.db, "gpt-test"); + + const form = new FormData(); + form.set("prompt", "build me a skill"); + form.set("mode", "simple"); + const res = await oh.app.request("/api/v1/skills/generate", { + method: "POST", + headers: buildAuth("u-multipart"), + body: form, + }); + expect(res.status).toBe(200); + const frames = await readFrames(res); + expect(frames.map((f) => f.type)).toEqual(["generation_start", "token", "generation_complete"]); + expect(streamCount()).toBe(1); + expect(completeCalls).toHaveLength(0); + } finally { + await oh.cleanup(); + } + }); +}); + +describe("IT-SKILLGEN-MODE-ADVANCED (default)", () => { + test("omitted mode accepts a scripted answer with references and assets", async () => { + const { client, completeCalls } = makeClient(SCRIPTED_JSON, PLAIN_JSON); + const oh = await startHarness({ llmClient: client }); + try { + await resetCollections(oh.db, ["quota_buckets", "platform_settings", "llm_providers"]); + await seedSkillGenModel(oh.db, "gpt-test"); + + const res = await oh.app.request("/api/v1/skills/generate", { + method: "POST", + headers: { ...buildAuth("u-advanced"), "Content-Type": "application/json" }, + body: JSON.stringify({ prompt: "build me a skill" }), + }); + expect(res.status).toBe(200); + const frames = await readFrames(res); + expect(frames.map((f) => f.type)).toEqual(["generation_start", "token", "generation_complete"]); + const raw = JSON.parse(String(frames[2]!.raw)) as { + scripts: unknown[]; + references: unknown[]; + assets: unknown[]; + }; + expect(raw.scripts).toHaveLength(1); + expect(raw.references).toHaveLength(1); + expect(raw.assets).toHaveLength(1); + expect(completeCalls).toHaveLength(0); + } finally { + await oh.cleanup(); + } + }); +}); From 130f9a62cba08a48029de85e93511d26e3ff9556 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:13:14 +0800 Subject: [PATCH 17/24] fix(api): classify simple-mode violations before schema check (#1242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found a hole in the simple-mode guarantee: the mode check ran only on a schema-VALID document, so a multi-turn answer that carried `scripts` / `references` / `assets` but tripped any other schema rule (over-long description, uppercase tag, …) was reported as `schema`, and the multi-turn path — which deliberately does not retry bad JSON — delivered it verbatim in `generation_complete`. The web preview would then show `scripts/` folders in Simple mode. `validateGeneratedSkill` now parses first, applies the simple-mode check on the raw object (any non-empty file/runtime array, a non-plain category, or an `outputType`), and only then runs the schema — so a files-carrying answer is a `mode_violation` no matter what else is wrong with it, and gets the corrective retry. Two more things the same review turned up in this module: - `outputType` is now a simple-mode violation. The comment claiming the frontmatter builder ignores it was wrong: the builder emits `output-type` and the frontmatter schema rejects it on a plain skill, so a stray value made the generated SKILL.md unpublishable. - The parser no longer throws for `readmeMd: null` / non-string `readmeMd`, or for JSON that is `null`, an array or a scalar (the legacy migration had moved outside the try block during the refactor). Those are `invalid_json` / `schema` rejections; an escaping TypeError would have killed the generator and ended the SSE stream with no terminal frame. Tests cover each case, including the multi-turn abort-after-first- answer guard that had no coverage. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../domains/skills/generation/service.test.ts | 33 +++++ .../skills/generation/validation.test.ts | 73 +++++++++- .../domains/skills/generation/validation.ts | 132 +++++++++++------- 3 files changed, 187 insertions(+), 51 deletions(-) diff --git a/ornn-api/src/domains/skills/generation/service.test.ts b/ornn-api/src/domains/skills/generation/service.test.ts index ea64997b..cc2e063a 100644 --- a/ornn-api/src/domains/skills/generation/service.test.ts +++ b/ornn-api/src/domains/skills/generation/service.test.ts @@ -712,6 +712,39 @@ describe("generateStreamWithHistory mode", () => { expect(err.message).toContain("retry 502"); expect(types(events)).not.toContain("generation_complete"); }); + + test("mode=simple: schema-invalid answer that carries files is retried, never delivered", async () => { + // Trips the schema (uppercase tag) AND carries scripts. The + // multi-turn no-retry rule for bad JSON must not apply here — the + // file-free guarantee wins. + const schemaInvalidScripted = JSON.stringify({ ...JSON.parse(SCRIPTED_SKILL), tags: ["Demo"] }); + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta(schemaInvalidScripted)], + completeResult: completeOutput(VALID_SKILL), + }); + const events = await drain(svc.generateStreamWithHistory(turn, { mode: "simple" })); + expect(types(events)).toEqual(["generation_start", "token", "validation_error", "generation_complete"]); + expect((events[2] as { retrying: boolean }).retrying).toBe(true); + expect(completeParams).toHaveLength(1); + expect((events[3] as { raw: string }).raw).toBe(VALID_SKILL); + }); + + test("mode=simple: abort flipped after the first answer skips the retry and ends in error", async () => { + const ctrl = new AbortController(); + const { svc, completeParams } = make({ + streamFrames: [outputTextDelta(SCRIPTED_SKILL)], + completeResult: completeOutput(VALID_SKILL), + // Abort after the last frame: the stream loop only re-checks the + // signal on the next iteration, so validation still runs, but the + // retry must be skipped. + onFrame: () => ctrl.abort(), + }); + const events = await drain( + svc.generateStreamWithHistory(turn, { mode: "simple", signal: ctrl.signal }), + ); + expect(types(events)).toEqual(["generation_start", "token", "validation_error", "error"]); + expect(completeParams).toHaveLength(0); + }); }); // ---- generateFromOpenApi --------------------------------------------- diff --git a/ornn-api/src/domains/skills/generation/validation.test.ts b/ornn-api/src/domains/skills/generation/validation.test.ts index 5cd57405..26a1dd5b 100644 --- a/ornn-api/src/domains/skills/generation/validation.test.ts +++ b/ornn-api/src/domains/skills/generation/validation.test.ts @@ -144,6 +144,7 @@ describe("findSimpleModeViolations", () => { test("names every offending field, category first", () => { expect(findSimpleModeViolations(parseGeneratedSkill(SCRIPTED_JSON)!)).toEqual([ "category", + "outputType", "scripts", "references", "assets", @@ -160,9 +161,19 @@ describe("findSimpleModeViolations", () => { expect(findSimpleModeViolations(skill)).toEqual(["references"]); }); - test("a stray outputType on a plain skill is tolerated", () => { + test("a stray outputType on a plain skill is a violation (frontmatter would reject it)", () => { const skill = parseGeneratedSkill(JSON.stringify({ ...PLAIN, outputType: "text" }))!; - expect(findSimpleModeViolations(skill)).toEqual([]); + expect(findSimpleModeViolations(skill)).toEqual(["outputType"]); + }); + + test("works on a raw parsed object that would fail the schema", () => { + // description too short, tag uppercase — but it still carries files. + const raw = { description: "x", tags: ["Bad"], scripts: [{ filename: "a.js", content: "1" }] }; + expect(findSimpleModeViolations(raw)).toEqual(["scripts"]); + }); + + test("ignores empty arrays and a missing category on a raw object", () => { + expect(findSimpleModeViolations({ scripts: [], references: [] })).toEqual([]); }); }); @@ -215,9 +226,63 @@ describe("validateGeneratedSkill", () => { } }); - test("a schema failure is reported as schema, not mode_violation", () => { - const r = validateGeneratedSkill(JSON.stringify({ ...SCRIPTED, name: "Bad Name" }), "simple"); + test("simple: a schema-invalid answer that still carries files is a mode_violation, not schema", () => { + // The multi-turn path delivers `schema` rejections verbatim (prose is + // allowed there), so a files-carrying answer must be classified as a + // mode violation regardless of the other schema rules it breaks. + for (const doc of [ + { ...SCRIPTED, name: "Bad Name" }, + { ...SCRIPTED, description: "x" }, + { ...SCRIPTED, tags: ["Demo"] }, + { description: "short", category: "runtime-based", scripts: [{ filename: "a.js", content: "1" }] }, + ]) { + const r = validateGeneratedSkill(JSON.stringify(doc), "simple"); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.reason).toBe("mode_violation"); + expect(r.violations).toContain("scripts"); + } + } + }); + + test("simple: a schema-invalid answer WITHOUT files is reported as schema", () => { + const r = validateGeneratedSkill(JSON.stringify({ ...PLAIN, name: "Bad Name" }), "simple"); expect(r.ok).toBe(false); if (!r.ok) expect(r.reason).toBe("schema"); }); + + test("advanced: a schema-invalid scripted answer is reported as schema", () => { + const r = validateGeneratedSkill(JSON.stringify({ ...SCRIPTED, name: "Bad Name" }), "advanced"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("schema"); + }); + + test("simple: outputType on an otherwise plain answer is a mode_violation", () => { + const r = validateGeneratedSkill(JSON.stringify({ ...PLAIN, outputType: "text" }), "simple"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.violations).toEqual(["outputType"]); + }); + + test("JSON that is null, an empty array or a scalar is invalid_json, never a throw", () => { + // (An array that CONTAINS an object is sliced down to that object by + // the brace-span cleanup — legacy behaviour, exercised elsewhere.) + for (const raw of ["null", "[]", "42", "\"str\"", "true"]) { + for (const mode of ["simple", "advanced"] as const) { + const r = validateGeneratedSkill(raw, mode); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("invalid_json"); + } + expect(parseGeneratedSkill(raw)).toBeNull(); + } + }); + + test("a non-string readmeMd is left to the schema instead of throwing", () => { + for (const readmeMd of [null, 42, { nested: true }]) { + const doc = { ...PLAIN, readmeBody: undefined, readmeMd }; + const r = validateGeneratedSkill(JSON.stringify(doc), "advanced"); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.reason).toBe("schema"); + expect(parseGeneratedSkill(JSON.stringify(doc))).toBeNull(); + } + }); }); diff --git a/ornn-api/src/domains/skills/generation/validation.ts b/ornn-api/src/domains/skills/generation/validation.ts index 95679090..cacb1356 100644 --- a/ornn-api/src/domains/skills/generation/validation.ts +++ b/ornn-api/src/domains/skills/generation/validation.ts @@ -61,10 +61,11 @@ export type GeneratedSkillValidation = }; /** - * Array fields that must be empty in `simple` mode. `category` is checked - * separately (must be `plain`). `outputType` is deliberately not listed: - * a stray `outputType` on a plain skill is harmless and the frontmatter - * builder ignores it. + * Array fields that must be empty in `simple` mode. `category` (must be + * `plain`) and `outputType` (must be absent — the web frontmatter + * builder emits `output-type` when set, and the frontmatter schema + * rejects it on a plain skill, so a stray value would make the + * generated SKILL.md unpublishable) are checked separately. */ const SIMPLE_MODE_EMPTY_FIELDS = [ "scripts", @@ -76,39 +77,42 @@ const SIMPLE_MODE_EMPTY_FIELDS = [ ] as const; /** - * Names of the fields that make a schema-valid skill unacceptable in - * `simple` mode. Empty array ⇒ the skill is a legal simple package. + * Names of the fields that make an answer unacceptable in `simple` + * mode. Works on the raw parsed JSON object as well as on a validated + * `GeneratedSkill`, so the check can run BEFORE schema validation — a + * document that trips an unrelated schema rule (say, an over-long + * description) but still carries `scripts` must be classified as a + * mode violation, not a schema failure, or the multi-turn path would + * deliver it verbatim. Empty array ⇒ legal simple package. */ -export function findSimpleModeViolations(skill: GeneratedSkill): string[] { +export function findSimpleModeViolations(doc: object): string[] { + const d = doc as Record; const violations: string[] = []; - if (skill.category !== "plain") violations.push("category"); + if (d.category !== undefined && d.category !== "plain") violations.push("category"); + if (d.outputType !== undefined && d.outputType !== null) violations.push("outputType"); for (const field of SIMPLE_MODE_EMPTY_FIELDS) { - if (skill[field].length > 0) violations.push(field); + const value = d[field]; + if (Array.isArray(value) && value.length > 0) violations.push(field); } return violations; } /** - * Strip markdown fences / surrounding prose, parse the JSON object and - * validate it against {@link generatedSkillSchema}. Returns `null` when - * the text is not a schema-valid skill document. + * Strip markdown fences / surrounding prose and parse the JSON object. + * Anything that is not a JSON object (unparseable text, `null`, an + * array, a scalar) is `invalid_json`. */ -export function parseGeneratedSkill(raw: string): GeneratedSkill | null { - const result = parseGeneratedSkillDetailed(raw); - return result.ok ? result.skill : null; -} - -function parseGeneratedSkillDetailed(raw: string): GeneratedSkillValidation { - let json: Record; - try { - let cleaned = raw.replace(/```json\n?/g, "").replace(/```\n?/g, "").trim(); +function parseJsonObject(raw: string): Record | null { + let cleaned = raw.replace(/```json\n?/g, "").replace(/```\n?/g, "").trim(); - const jsonStart = cleaned.indexOf("{"); - const jsonEnd = cleaned.lastIndexOf("}"); - if (jsonStart >= 0 && jsonEnd > jsonStart) { - cleaned = cleaned.slice(jsonStart, jsonEnd + 1); - } + const jsonStart = cleaned.indexOf("{"); + const jsonEnd = cleaned.lastIndexOf("}"); + if (jsonStart >= 0 && jsonEnd > jsonStart) { + cleaned = cleaned.slice(jsonStart, jsonEnd + 1); + } + let json: unknown; + try { json = JSON.parse(cleaned); } catch (err) { // Generated-skill JSON parse failed. Caller treats this as @@ -116,12 +120,28 @@ function parseGeneratedSkillDetailed(raw: string): GeneratedSkillValidation { // so we can spot a model that's consistently producing // unparseable output (#579). logger.debug({ err }, "generated skill JSON parse failed"); - return { ok: false, reason: "invalid_json", message: "Invalid JSON from LLM", violations: [] }; + return null; } + if (json === null || typeof json !== "object" || Array.isArray(json)) { + logger.debug({ kind: Array.isArray(json) ? "array" : typeof json }, "generated skill JSON is not an object"); + return null; + } + return json as Record; +} - // Handle backward-compat: rename readmeMd -> readmeBody - if (json.readmeMd && !json.readmeBody) { - const md = json.readmeMd as string; +const INVALID_JSON: GeneratedSkillValidation = { + ok: false, + reason: "invalid_json", + message: "Invalid JSON from LLM", + violations: [], +}; + +/** Schema-validate a parsed object, applying the legacy `readmeMd` migration first. */ +function validateSchema(json: Record): GeneratedSkillValidation { + // Handle backward-compat: rename readmeMd -> readmeBody. Only a string + // can be migrated; anything else is left for the schema to reject. + if (typeof json.readmeMd === "string" && !json.readmeBody) { + const md = json.readmeMd; const fmEnd = md.indexOf("\n---", 3); json.readmeBody = fmEnd > 0 ? md.slice(fmEnd + 4).trim() : md; delete json.readmeMd; @@ -142,26 +162,44 @@ function parseGeneratedSkillDetailed(raw: string): GeneratedSkillValidation { } /** - * Parse + schema-validate, then apply the package-shape rule for `mode`. - * In `advanced` mode every schema-valid answer is accepted; in `simple` - * mode an answer that carries scripts / references / assets / runtime - * fields or a non-plain category is rejected as a `mode_violation`. + * Parse + schema-validate. Returns `null` when the text is not a + * schema-valid skill document. + */ +export function parseGeneratedSkill(raw: string): GeneratedSkill | null { + const json = parseJsonObject(raw); + if (!json) return null; + const result = validateSchema(json); + return result.ok ? result.skill : null; +} + +/** + * Parse, then apply the package-shape rule for `mode`, then + * schema-validate. In `advanced` mode every schema-valid answer is + * accepted; in `simple` mode any parseable answer that carries scripts / + * references / assets / runtime fields, an `outputType`, or a non-plain + * category is rejected as a `mode_violation` — before the schema runs, + * so the classification does not depend on the rest of the document + * being well-formed. */ export function validateGeneratedSkill( raw: string, mode: GenerationMode, ): GeneratedSkillValidation { - const parsed = parseGeneratedSkillDetailed(raw); - if (!parsed.ok || mode !== "simple") return parsed; - - const violations = findSimpleModeViolations(parsed.skill); - if (violations.length === 0) return parsed; - - logger.debug({ violations }, "Generated skill violates simple mode"); - return { - ok: false, - reason: "mode_violation", - message: `Simple mode allows SKILL.md only, but the model emitted: ${violations.join(", ")}`, - violations, - }; + const json = parseJsonObject(raw); + if (!json) return INVALID_JSON; + + if (mode === "simple") { + const violations = findSimpleModeViolations(json); + if (violations.length > 0) { + logger.debug({ violations }, "Generated skill violates simple mode"); + return { + ok: false, + reason: "mode_violation", + message: `Simple mode allows SKILL.md only, but the model emitted: ${violations.join(", ")}`, + violations, + }; + } + } + + return validateSchema(json); } From e9e26f8c7a1957992a009341b8923bcd5693c21c Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:13:49 +0800 Subject: [PATCH 18/24] =?UTF-8?q?test(api):=20pin=20repeated=20multipart?= =?UTF-8?q?=20mode=20field=20=E2=86=92=20invalid=5Fmode=20(#1242)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route doc promised that a repeated `mode` form field (which `parseBody({ all: true })` surfaces as a string array) is rejected like an unknown string, but nothing exercised it. Reproduced against the mounted routes: two `mode` parts → 400 `invalid_mode`, quota never reserved. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../src/domains/skills/generation/routes.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ornn-api/src/domains/skills/generation/routes.test.ts b/ornn-api/src/domains/skills/generation/routes.test.ts index f10c900f..5266a175 100644 --- a/ornn-api/src/domains/skills/generation/routes.test.ts +++ b/ornn-api/src/domains/skills/generation/routes.test.ts @@ -541,6 +541,19 @@ describe("POST /skills/generate — mode", () => { expect(quota.checkAllowedCalls).toBe(0); }); + it("rejects a repeated multipart mode field (array under parseBody all:true) with invalid_mode", async () => { + const quota = new FakeQuotaService(); + const { app } = buildApp({ quotaService: quota }); + const form = new FormData(); + form.set("prompt", "p"); + form.append("mode", "simple"); + form.append("mode", "advanced"); + const res = await app.request("/api/v1/skills/generate", { method: "POST", body: form }); + expect(res.status).toBe(400); + expect(((await res.json()) as { code: string }).code).toBe("invalid_mode"); + expect(quota.checkAllowedCalls).toBe(0); + }); + it("rejects a multipart mode sent as a file with invalid_mode", async () => { const { app } = buildApp(); const form = new FormData(); From f2d57198e2999412ab4866e5b323b3e920807e1d Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:14:02 +0800 Subject: [PATCH 19/24] fix(web): move focus with arrow keys in GenerationModeToggle (#1242) With a roving tabindex, the previously focused segment drops to `tabIndex=-1` as soon as the selection changes, so an arrow key that changed the value but left focus behind stranded keyboard users on an untabbable button. Each segment now keeps a ref and `move()` focuses the newly selected one. The test that should have caught this was vacuous: it asserted `toHaveBeenLastCalledWith("advanced")` twice in a row, so the wrap case passed even if the second keypress did nothing. It now clears the mock between keypresses, asserts the call count, and checks `document.activeElement` after an arrow key. Part of #1242. Claude-Session: https://claude.ai/code/session_01Pi6Ymxei9vAupEWmt3gjxh --- .../generative/GenerationModeToggle.test.tsx | 28 ++++++++++++++++--- .../skill/generative/GenerationModeToggle.tsx | 9 ++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx b/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx index 6b91383d..586fba83 100644 --- a/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx +++ b/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx @@ -2,8 +2,9 @@ * UT-WEB-GENERATION-MODE-TOGGLE-001 (#1242) * * Pins the SIMPLE | ADVANCED segmented control: ARIA radiogroup - * semantics, click + arrow-key selection, roving tabindex, - * and the disabled lock used while streaming. + * semantics, click + arrow-key selection (which also moves focus, or + * the roving tabindex would strand the keyboard user), and the + * disabled lock used while streaming. * * @module components/skill/generative/GenerationModeToggle.test */ @@ -63,20 +64,39 @@ describe("GenerationModeToggle", () => { const onChange = vi.fn(); const { rerender } = render(); const group = screen.getByRole("radiogroup"); + fireEvent.keyDown(group, { key: "ArrowRight" }); + expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenLastCalledWith("advanced"); + + // From simple, Left wraps to advanced — a distinct call, not the previous one. + onChange.mockClear(); fireEvent.keyDown(group, { key: "ArrowLeft" }); - // From simple, Left wraps to advanced. + expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenLastCalledWith("advanced"); rerender(); + onChange.mockClear(); fireEvent.keyDown(group, { key: "ArrowDown" }); - // From advanced, Down wraps to simple. + expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenLastCalledWith("simple"); + + onChange.mockClear(); fireEvent.keyDown(group, { key: "ArrowUp" }); + expect(onChange).toHaveBeenCalledTimes(1); expect(onChange).toHaveBeenLastCalledWith("simple"); }); + it("arrow keys move focus to the newly selected segment (roving tabindex)", () => { + const onChange = vi.fn(); + render(); + const [simple, advanced] = radios(); + simple.focus(); + expect(document.activeElement).toBe(simple); + fireEvent.keyDown(screen.getByRole("radiogroup"), { key: "ArrowRight" }); + expect(document.activeElement).toBe(advanced); + }); + it("ignores unrelated keys", () => { const onChange = vi.fn(); render(); diff --git a/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx b/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx index d1a6f128..515865fc 100644 --- a/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx +++ b/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx @@ -18,6 +18,7 @@ * @module components/skill/generative/GenerationModeToggle */ +import { useRef } from "react"; import { useTranslation } from "react-i18next"; import { GENERATION_MODES, type GenerationMode } from "@/types/skillPackage"; import { useGenerationModeCopy } from "@/hooks/useGenerationMode"; @@ -38,11 +39,16 @@ export function GenerationModeToggle({ }: GenerationModeToggleProps) { const { t } = useTranslation(); const { labels, hints } = useGenerationModeCopy(); + const segmentRefs = useRef>>({}); + // Arrow keys both select AND move focus: with a roving tabindex the + // previously focused segment drops to tabIndex -1 on re-render, so + // leaving focus there would strand the keyboard user. const move = (delta: 1 | -1) => { const idx = GENERATION_MODES.indexOf(value); const next = GENERATION_MODES[(idx + delta + GENERATION_MODES.length) % GENERATION_MODES.length]!; if (next !== value) onChange(next); + segmentRefs.current[next]?.focus(); }; const handleKeyDown = (e: React.KeyboardEvent) => { @@ -75,6 +81,9 @@ export function GenerationModeToggle({ return (