diff --git a/.changeset/audit-transitive-overrides.md b/.changeset/audit-transitive-overrides.md new file mode 100644 index 00000000..a845151c --- /dev/null +++ b/.changeset/audit-transitive-overrides.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.changeset/release-notes-20260917.md b/.changeset/release-notes-20260917.md new file mode 100644 index 00000000..a845151c --- /dev/null +++ b/.changeset/release-notes-20260917.md @@ -0,0 +1,2 @@ +--- +--- 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. diff --git a/.github/release-notes-20260917.md b/.github/release-notes-20260917.md new file mode 100644 index 00000000..798848f9 --- /dev/null +++ b/.github/release-notes-20260917.md @@ -0,0 +1,17 @@ +## Fixed + +- Rejected skill-generation requests now show the server's actual reason +- Few technical bugs fixed + +## New Feature + +- Choose Simple or Advanced mode when generating a skill +- Simple mode returns one SKILL.md, enforced by the server +- Advanced generation can now include references and assets files +- Agents can request the generation mode through the API + +## Changed + +- Generative builder gains a mode toggle beside the model picker +- API reference and agent manuals describe generation modes +- Technical enhancement diff --git a/bun.lock b/bun.lock index 6deeb38b..3fcf4187 100644 --- a/bun.lock +++ b/bun.lock @@ -16,7 +16,7 @@ }, "ornn-api": { "name": "ornn-api", - "version": "0.16.0", + "version": "0.17.0", "dependencies": { "@agendajs/mongo-backend": "^4.0.2", "agenda": "^6.2.5", @@ -39,7 +39,7 @@ }, "ornn-web": { "name": "ornn-web", - "version": "0.16.0", + "version": "0.17.0", "dependencies": { "@hookform/resolvers": "^5.4.0", "@tanstack/react-query": "^5.101.2", @@ -97,13 +97,16 @@ }, "overrides": { "brace-expansion": "^5.0.8", + "browserslist": "^4.28.7", "dompurify": "^3.3.4", "eslint": "10.2.0", "eslint-plugin-react-hooks": "7.0.1", - "js-yaml": "^4.3.0", + "js-yaml": "^4.3.2", + "nanoid": "^3.3.18", "picomatch": "^4.0.4", "postcss": "^8.5.18", "undici": "^7.29.0", + "update-browserslist-db": "^1.2.4", "uuid": "^14.0.0", }, "packages": { @@ -375,7 +378,7 @@ "@types/aria-query": ["@types/aria-query@5.0.4", "", {}, "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw=="], - "@types/bun": ["@types/bun@1.3.14", "", { "dependencies": { "bun-types": "1.3.14" } }, "sha512-h1hFqFVcvAvD9j9K7ZW7vd82aSA+rTdznZa+5bwvCwqSB1jmmfLcbIWhOLx1/+boy/xmjgCs/OMUL8hRJSmnPw=="], + "@types/bun": ["@types/bun@1.4.2", "", { "dependencies": { "bun-types": "1.4.2" } }, "sha512-GimotNn7+ZV0uVArItBbriZsR1oNf0+WTzPkdcFrzShI7k2norL0uzEaJT8T33dWr7O/c9ZDuAFQrctKCi72oQ=="], "@types/chai": ["@types/chai@5.2.3", "", { "dependencies": { "@types/deep-eql": "*", "assertion-error": "^2.0.1" } }, "sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA=="], @@ -575,7 +578,7 @@ "bare-url": ["bare-url@2.4.3", "", { "dependencies": { "bare-path": "^3.0.0" } }, "sha512-Kccpc7ACfXaxfeInfqKcZtW4pT5YBn1mesc4sCsun6sRwtbJ4h+sNOaksUpYEJUKfN65YWC6Bw2OJEFiKxq8nQ=="], - "baseline-browser-mapping": ["baseline-browser-mapping@2.10.29", "", { "bin": { "baseline-browser-mapping": "dist/cli.cjs" } }, "sha512-Asa2krT+XTPZINCS+2QcyS8WTkObE77RwkydwF7h6DmnKqbvlalz93m/dnphUyCa6SWSP51VgtEUf2FN+gelFQ=="], + "baseline-browser-mapping": ["baseline-browser-mapping@2.11.24", "", { "bin": { "baseline-browser-mapping": "dist/cli.cjs" } }, "sha512-hYrgxie335U08WqICoGqKRzV1HFXv6zdxwJE4ekCb80CM9a0SVVsN4QPwT67RraRo+9h8IATk6uxHJw7QSkdOg=="], "better-path-resolve": ["better-path-resolve@1.0.0", "", { "dependencies": { "is-windows": "^1.0.0" } }, "sha512-pbnl5XzGBdrFU/wT4jqmJVPn2B6UHPBOhzMQkY/SPUPB6QtUXtmBHBIwCbXJol93mOpGMnQyP/+BB19q04xj7g=="], @@ -585,7 +588,7 @@ "braces": ["braces@3.0.3", "", { "dependencies": { "fill-range": "^7.1.1" } }, "sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA=="], - "browserslist": ["browserslist@4.28.2", "", { "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", "electron-to-chromium": "^1.5.328", "node-releases": "^2.0.36", "update-browserslist-db": "^1.2.3" }, "bin": { "browserslist": "cli.js" } }, "sha512-48xSriZYYg+8qXna9kwqjIVzuQxi+KYWp2+5nCYnYKPTr0LvD89Jqk2Or5ogxz0NUMfIjhh2lIUX/LyX9B4oIg=="], + "browserslist": ["browserslist@4.29.0", "", { "dependencies": { "baseline-browser-mapping": "^2.11.23", "caniuse-lite": "^1.0.30001810", "electron-to-chromium": "^1.5.427", "node-releases": "^2.0.55", "update-browserslist-db": "^1.3.3" }, "bin": { "browserslist": "cli.js" } }, "sha512-3GSvyjvDI4Dur1Meg2BekJquu5uF+9R9a1+5M1Mde192eZoXbeXjzgOsgqPS2V8D5wrrip0gR5Hf/GhWQ9ZzaA=="], "bson": ["bson@7.2.0", "", {}, "sha512-YCEo7KjMlbNlyHhz7zAZNDpIpQbd+wOEHJYezv0nMYTn4x31eIUM2yomNNubclAt63dObUzKHWsBLJ9QcZNSnQ=="], @@ -593,7 +596,7 @@ "camelcase": ["camelcase@6.3.0", "", {}, "sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA=="], - "caniuse-lite": ["caniuse-lite@1.0.30001792", "", {}, "sha512-hVLMUZFgR4JJ6ACt1uEESvQN1/dBVqPAKY0hgrV70eN3391K6juAfTjKZLKvOMsx8PxA7gsY1/tLMMTcfFLLpw=="], + "caniuse-lite": ["caniuse-lite@1.0.30001810", "", {}, "sha512-TITQPUkaz+aVk5GL6NhOdwk1aEaNTSDPsGFWrTuhKGtjTF70jL/Oht2W4c6rXUe5fu7Ie19VIahAXHIIiWWNeg=="], "ccount": ["ccount@2.0.1", "", {}, "sha512-eyrF0jiFpY+3drT6383f1qhkbGsLSifNAjA61IUjZjmLCWjItY6LB9ft9YhoDgwfmclB2zhu51Lc7+95b8NRAg=="], @@ -753,7 +756,7 @@ "dotenv": ["dotenv@8.6.0", "", {}, "sha512-IrPdXQsk2BbzvCBGBOTmmSH5SodmqZNt4ERAZDmW4CT+tL8VtvinqywuANaFu4bOMWki16nqf0e4oC0QIaDr/g=="], - "electron-to-chromium": ["electron-to-chromium@1.5.353", "", {}, "sha512-kOrWphBi8TOZyiJZqsgqIle0lw+tzmnQK83pV9dZUd01Nm2POECSyFQMAuarzZdYqQW7FH9RaYOuaRo3h+bQ3w=="], + "electron-to-chromium": ["electron-to-chromium@1.5.430", "", {}, "sha512-e1QEj72Y4zd8RlNZVmoTg+iCOSVwpk05IOiiQwdrkwCSVlZfPthevErhE+nckGd2YbsXfp1SkisznhGVIXP2NQ=="], "end-of-stream": ["end-of-stream@1.4.5", "", { "dependencies": { "once": "^1.4.0" } }, "sha512-ooEGc6HP26xXq/N+GCGOT0JKCLDGrq2bQUZrQ7gyrJiZANJ/8YDTxTpQBXGMn+WbIQXNVpyWymm7KYVICQnyOg=="], @@ -951,7 +954,7 @@ "js-tokens": ["js-tokens@10.0.0", "", {}, "sha512-lM/UBzQmfJRo9ABXbPWemivdCW8V2G8FHaHdypQaIy523snUjog0W71ayWXTjiR+ixeMyVHN2XcpnTd/liPg/Q=="], - "js-yaml": ["js-yaml@4.3.1", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ=="], + "js-yaml": ["js-yaml@4.3.2", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA=="], "jsdom": ["jsdom@29.1.1", "", { "dependencies": { "@asamuzakjp/css-color": "^5.1.11", "@asamuzakjp/dom-selector": "^7.1.1", "@bramus/specificity": "^2.4.2", "@csstools/css-syntax-patches-for-csstree": "^1.1.3", "@exodus/bytes": "^1.15.0", "css-tree": "^3.2.1", "data-urls": "^7.0.0", "decimal.js": "^10.6.0", "html-encoding-sniffer": "^6.0.0", "is-potential-custom-element-name": "^1.0.1", "lru-cache": "^11.3.5", "parse5": "^8.0.1", "saxes": "^6.0.0", "symbol-tree": "^3.2.4", "tough-cookie": "^6.0.1", "undici": "^7.25.0", "w3c-xmlserializer": "^5.0.0", "webidl-conversions": "^8.0.1", "whatwg-mimetype": "^5.0.0", "whatwg-url": "^16.0.1", "xml-name-validator": "^5.0.0" }, "peerDependencies": { "canvas": "^3.0.0" }, "optionalPeers": ["canvas"] }, "sha512-ECi4Fi2f7BdJtUKTflYRTiaMxIB0O6zfR1fX0GXpUrf6flp8QIYn1UT20YQqdSOfk2dfkCwS8LAFoJDEppNK5Q=="], @@ -1149,7 +1152,7 @@ "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], - "nanoid": ["nanoid@3.3.17", "", { "bin": { "nanoid": "bin/nanoid.cjs" } }, "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g=="], + "nanoid": ["nanoid@3.3.19", "", { "bin": { "nanoid": "bin/nanoid.cjs" } }, "sha512-Y2tUNy4ouw6tq5oDSKeQYGOyhkUBhNOcGV/02KC+6kd9eDGqdZd++mjMiIDilrBYvjEnCYvVtsuHCuP+okSfug=="], "natural-compare": ["natural-compare@1.4.0", "", {}, "sha512-OWND8ei3VtNC9h7V60qff3SVobHr996CTwgxubgyQYEpg290h9J0buyECNNJexkFm5sOajh5G116RYA1c8ZMSw=="], @@ -1157,7 +1160,7 @@ "node-fetch": ["node-fetch@2.7.0", "", { "dependencies": { "whatwg-url": "^5.0.0" }, "peerDependencies": { "encoding": "^0.1.0" }, "optionalPeers": ["encoding"] }, "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A=="], - "node-releases": ["node-releases@2.0.44", "", {}, "sha512-5WUyunoPMsvvEhS8AxHtRzP+oA8UCkJ7YRxatWKjngndhDGLiqEVAQKWjFAiAiuL8zMRGzGSJxFnLetoa43qGQ=="], + "node-releases": ["node-releases@2.0.55", "", {}, "sha512-mIrE/Cw9y+9Au6dS5vDKDhQza9YvG6w+ZrS6X+ZzA7yFW/soAeaups4Qzn1bL6g5FVy8WtP79+0j82oPIbqRjQ=="], "numbered": ["numbered@1.1.0", "", {}, "sha512-pv/ue2Odr7IfYOO0byC1KgBI10wo5YDauLhxY6/saNzAdAs0r1SotGCPzzCLNPL0xtrAwWRialLu23AAu9xO1g=="], @@ -1457,7 +1460,7 @@ "universalify": ["universalify@0.1.2", "", {}, "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg=="], - "update-browserslist-db": ["update-browserslist-db@1.2.3", "", { "dependencies": { "escalade": "^3.2.0", "picocolors": "^1.1.1" }, "peerDependencies": { "browserslist": ">= 4.21.0" }, "bin": { "update-browserslist-db": "cli.js" } }, "sha512-Js0m9cx+qOgDxo0eMiFGEueWztz+d4+M3rGlmKPT+T4IS/jP4ylw3Nwpu6cpTTP8R1MAC1kF4VbdLt3ARf209w=="], + "update-browserslist-db": ["update-browserslist-db@1.3.3", "", { "dependencies": { "escalade": "^3.2.0", "picocolors": "^1.1.1" }, "peerDependencies": { "browserslist": ">= 4.21.0" }, "bin": { "update-browserslist-db": "cli.js" } }, "sha512-pJ2sYawQS0R/WI928Gj5GlPhTGzbMelq0+4INtSYNDV9ErKJcX6xjGWkoG/VnB3dpUm00zALaqkrUD77pO5TDQ=="], "uri-js": ["uri-js@4.4.1", "", { "dependencies": { "punycode": "^2.1.0" } }, "sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg=="], @@ -1559,6 +1562,8 @@ "@testing-library/dom/dom-accessibility-api": ["dom-accessibility-api@0.5.16", "", {}, "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg=="], + "@types/bun/bun-types": ["bun-types@1.4.2", "", { "dependencies": { "@types/node": "*" } }, "sha512-bxV1FgK7yBIzjRe5zBozIM4Bem11ZJcCXSrjWRG3YWLt8yFDePu4cLjpebO8OvPeIE9trbyPF4fuj3Cia4Fj3w=="], + "@typescript-eslint/eslint-plugin/ignore": ["ignore@7.0.5", "", {}, "sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg=="], "@xyflow/react/zustand": ["zustand@4.5.7", "", { "dependencies": { "use-sync-external-store": "^1.2.2" }, "peerDependencies": { "@types/react": ">=16.8", "immer": ">=9.0.6", "react": ">=16.8" }, "optionalPeers": ["@types/react", "immer", "react"] }, "sha512-CHOUy7mu3lbD6o6LJLfllpjkzhHXSBlX8B9+qPddUsIfeF5S/UZ5q0kmCsnRqT1UHFQZchNFDDzMbQsuesHWlw=="], 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/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/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}"`, }; } diff --git a/ornn-api/src/domains/skills/generation/routes.test.ts b/ornn-api/src/domains/skills/generation/routes.test.ts index 2f97238c..5266a175 100644 --- a/ornn-api/src/domains/skills/generation/routes.test.ts +++ b/ornn-api/src/domains/skills/generation/routes.test.ts @@ -35,9 +35,10 @@ 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"; +import type { GenerationMode, SkillStreamEvent } from "../../../shared/types/index"; import type { ChargeOutcome } from "../../quota/types"; import type { ModelResolution } from "../../settings/llmProviders/service"; @@ -82,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; @@ -97,17 +102,21 @@ class FakeGenerationService { generateStream( query: string, - _signal?: AbortSignal, - modelOverride?: string, + options: GenerateOptions = {}, ): AsyncIterable { - this.generateStreamCalls.push({ query, 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,134 @@ 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 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(); + 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 296e3ccd..d816394b 100644 --- a/ornn-api/src/domains/skills/generation/routes.ts +++ b/ornn-api/src/domains/skills/generation/routes.ts @@ -5,26 +5,26 @@ */ 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, requirePermission, getAuth, } from "../../../middleware/nyxidAuth"; -import { AppError } from "../../../shared/types/index"; -import { resolveZipRoot } from "../../../shared/utils/zip"; +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"; -import JSZip from "jszip"; +import { analyzePackageContent } from "./packageContext"; +import { preflight, resolveKeepAliveMs, streamGenerationEvents } from "./streaming"; import { createLogger } from "../../../shared/logger"; import { z } from "zod"; @@ -37,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; /** @@ -52,195 +74,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 }>(); @@ -249,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 */ @@ -268,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 }); @@ -280,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) { @@ -290,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); @@ -305,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)) { @@ -323,15 +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 }>, - c.req.raw.signal, - 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 }, @@ -360,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, pf.modelId), + generationService.generateStream(query, { signal, modelOverride: pf.modelId, mode }), 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 d420266b..cc2e063a 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,116 @@ 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"); + }); + + 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 --------------------------------------------- describe("generateFromOpenApi", () => { @@ -591,90 +826,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(); - }); -}); diff --git a/ornn-api/src/domains/skills/generation/service.ts b/ornn-api/src/domains/skills/generation/service.ts index d2ca1e49..318a45bc 100644 --- a/ornn-api/src/domains/skills/generation/service.ts +++ b/ornn-api/src/domains/skills/generation/service.ts @@ -5,37 +5,32 @@ * @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 { + 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, + validateGeneratedSkill, + type GeneratedSkillValidation, +} 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 }; +/** 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` @@ -61,6 +56,24 @@ export interface GenerationServiceConfig { defaultsResolver: SkillGenLlmDefaultsResolver; } +/** Resolved per-call LLM parameters shared by every generator. */ +interface LlmCallContext { + model: string; + 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; @@ -81,51 +94,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,88 +156,169 @@ 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; } - // 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 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 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; + 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; + } + + /** + * 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; + } - yield { type: "error", message: "LLM produced invalid output after retry" }; + /** 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; } - yield { type: "generation_complete", raw: accumulated }; + 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) }; } /** - * Multi-turn generation. Converts message history to Responses API format. + * 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. `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 *generateStreamWithHistory( - messages: Array<{ role: "user" | "assistant"; content: string }>, - signal?: AbortSignal, - modelOverride?: string, + async *generateStream( + query: string, + options: GenerateOptions = {}, ): AsyncIterable { - let defaults: SkillGenLlmDefaults; - try { - defaults = await this.resolveDefaults(); - } catch (err) { - yield { type: "error", message: (err as Error).message }; + const { signal, modelOverride } = options; + const mode = options.mode ?? DEFAULT_GENERATION_MODE; + const ctx = yield* this.begin(signal, modelOverride); + if (!ctx) return; + + const { instructions, userPrompt } = buildDirectGenerationPrompt(query, mode); + const input: ResponsesApiInputMessage[] = [ + { role: "developer", content: instructions }, + { role: "user", content: userPrompt }, + ]; + + const accumulated = yield* this.streamLlm(input, ctx, signal, "direct"); + if (accumulated === null) return; + + const validation = validateGeneratedSkill(accumulated, mode); + if (validation.ok) { + yield { type: "generation_complete", raw: accumulated }; return; } - const model = modelOverride ?? defaults.model; + + logger.warn( + { mode, reason: validation.reason, violations: validation.violations }, + "LLM output failed validation, attempting retry", + ); + yield { type: "validation_error", message: validation.message, retrying: true }; + if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; + yield { type: "error", message: SkillGenerationService.exhaustedMessageFor(validation) }; return; } - yield { type: "generation_start" }; + 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 }>, + 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 { @@ -232,49 +333,49 @@ export class SkillGenerationService { }), ]; - let accumulated = ""; + const accumulated = yield* this.streamLlm(input, ctx, signal, "multi-turn"); + if (accumulated === null) return; - try { - const streamEvents = this.llmClient.stream({ - model, - input, - max_output_tokens: defaults.maxOutputTokens, - temperature: defaults.temperature, - }); + logger.info( + { mode, accumulatedLength: accumulated.length, first200: accumulated.slice(0, 200), last200: accumulated.slice(-200) }, + "Multi-turn generation accumulated text", + ); - for await (const event of streamEvents) { - if (signal?.aborted) { - yield { type: "error", message: "Request aborted" }; - return; - } + 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; + } - 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}` }; + 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.info( - { accumulatedLength: accumulated.length, first200: accumulated.slice(0, 200), last200: accumulated.slice(-200) }, - "Multi-turn generation accumulated text", + logger.warn( + { mode, violations: validation.violations }, + "Multi-turn output violates simple mode, attempting retry", ); + yield { type: "validation_error", message: validation.message, retrying: true }; - 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"); + if (signal?.aborted) { + yield { type: "error", message: SkillGenerationService.exhaustedMessageFor(validation) }; + return; } - yield { type: "generation_complete", raw: accumulated }; + // 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); } /** @@ -287,20 +388,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,36 +397,10 @@ 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); + const parsed = parseGeneratedSkill(accumulated); if (!parsed) { logger.warn("OpenAPI generation output failed validation"); yield { type: "validation_error", message: "Invalid JSON from LLM", retrying: false }; @@ -366,20 +429,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,36 +438,10 @@ 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 accumulated = yield* this.streamLlm(input, ctx, signal, "Source-code generation"); + if (accumulated === null) 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 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 }; @@ -425,47 +450,6 @@ export class SkillGenerationService { yield { type: "generation_complete", raw: accumulated }; } - 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; - } - } } /** 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", + ); + }); + } + } + }); +} 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 }; 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..26a1dd5b --- /dev/null +++ b/ornn-api/src/domains/skills/generation/validation.test.ts @@ -0,0 +1,288 @@ +/** + * 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", + "outputType", + "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 a violation (frontmatter would reject it)", () => { + const skill = parseGeneratedSkill(JSON.stringify({ ...PLAIN, outputType: "text" }))!; + 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([]); + }); +}); + +// ---- 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("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 new file mode 100644 index 00000000..cacb1356 --- /dev/null +++ b/ornn-api/src/domains/skills/generation/validation.ts @@ -0,0 +1,205 @@ +/** + * Parsing + schema validation of the JSON document the LLM returns for + * 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, GenerationMode } from "../../../shared/types/index"; +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), + 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(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([]), +}); + +/** + * 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` (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", + "references", + "assets", + "runtimes", + "dependencies", + "envVars", +] as const; + +/** + * 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(doc: object): string[] { + const d = doc as Record; + const violations: string[] = []; + 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) { + const value = d[field]; + if (Array.isArray(value) && value.length > 0) violations.push(field); + } + return violations; +} + +/** + * 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`. + */ +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); + } + + let json: unknown; + try { + json = JSON.parse(cleaned); + } catch (err) { + // 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; + } + 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; +} + +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; + } + + 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. 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 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); +} diff --git a/ornn-api/src/openapi/paths/generation.ts b/ornn-api/src/openapi/paths/generation.ts index 1677c31a..fd63bff2 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\"` without an `outputType` and with each of `scripts`, `references`, `assets`, `runtimes`, `dependencies` and `envVars` empty or absent — so `generation_complete.raw` in simple mode is guaranteed file-free. Omit the field, or send `null` or 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. `raw` is the model's verbatim answer, so any of these arrays may be omitted; treat a missing array as empty. 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 — a parseable answer that carries files, an `outputType` or a non-plain category, whatever else the schema says about it — which gets the same single corrective retry and ends on `error` unless the retry is a valid, file-free skill. `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 prompt asks for a `plain`, file-free skill (the shape `simple` mode produces) but, unlike `simple`, this path does not enforce that server-side — validate `raw` before publishing. 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 prompt asks for a `plain`, file-free skill (the shape `simple` mode produces) but the server does not enforce that on this path — validate `raw` before publishing. A `mode` key in the body is ignored, not rejected. " + GENERATION_STREAM_CONTRACT + " " + GENERATION_COST_CONTRACT + 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 = 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..6950b110 --- /dev/null +++ b/ornn-api/tests/integration/skillgen_mode.test.ts @@ -0,0 +1,340 @@ +/** + * 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); + +/** + * Tests that boot their own harness (fresh app + MongoMemoryServer, to + * inject the LLM double) need more than Bun's 5 s default under CI load + * — one such test timed out at 5005 ms on a shared runner. Matches the + * 30 s the `afterAll` cleanup already gets. + */ +const HARNESS_TEST_TIMEOUT_MS = 30_000; + +/** 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(); + } + }, HARNESS_TEST_TIMEOUT_MS); + + 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(); + } + }, HARNESS_TEST_TIMEOUT_MS); + + 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(); + } + }, HARNESS_TEST_TIMEOUT_MS); +}); + +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(); + } + }, HARNESS_TEST_TIMEOUT_MS); +}); diff --git a/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx b/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx new file mode 100644 index 00000000..586fba83 --- /dev/null +++ b/ornn-web/src/components/skill/generative/GenerationModeToggle.test.tsx @@ -0,0 +1,119 @@ +/** + * UT-WEB-GENERATION-MODE-TOGGLE-001 (#1242) + * + * Pins the SIMPLE | ADVANCED segmented control: ARIA radiogroup + * 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 + */ + +import { describe, it, expect, vi, afterEach } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { GenerationModeToggle } from "./GenerationModeToggle"; + +afterEach(() => cleanup()); + +function radios() { + return screen.getAllByRole("radio") as HTMLButtonElement[]; +} + +describe("GenerationModeToggle", () => { + it("renders a labelled radiogroup with Simple and Advanced segments", () => { + render( {}} />); + const group = screen.getByRole("radiogroup", { name: "Generation mode" }); + expect(group).toBeInTheDocument(); + const [simple, advanced] = radios(); + expect(simple).toHaveTextContent("Simple"); + expect(advanced).toHaveTextContent("Advanced"); + expect(simple).toHaveAttribute("aria-checked", "false"); + expect(advanced).toHaveAttribute("aria-checked", "true"); + // Each segment explains itself for screen readers and via title. + expect(simple).toHaveAttribute("title", "SKILL.md only — no scripts, references or assets"); + expect(advanced.getAttribute("aria-label")).toContain("scripts, references and assets"); + }); + + it("marks only the selected segment with the ember treatment", () => { + render( {}} />); + const [simple, advanced] = radios(); + expect(simple.className).toContain("border-accent"); + expect(simple.className).toContain("text-accent"); + expect(advanced.className).toContain("border-transparent"); + expect(advanced.className).not.toContain("text-accent"); + }); + + it("clicking the other segment calls onChange; clicking the selected one does not", () => { + const onChange = vi.fn(); + render(); + const [simple, advanced] = radios(); + fireEvent.click(advanced); + expect(onChange).not.toHaveBeenCalled(); + fireEvent.click(simple); + expect(onChange).toHaveBeenCalledWith("simple"); + }); + + it("uses a roving tabindex so only the selected segment is tabbable", () => { + render( {}} />); + const [simple, advanced] = radios(); + expect(simple.tabIndex).toBe(0); + expect(advanced.tabIndex).toBe(-1); + }); + + it("arrow keys move the selection and wrap around", () => { + 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" }); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith("advanced"); + + rerender(); + onChange.mockClear(); + fireEvent.keyDown(group, { key: "ArrowDown" }); + 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(); + fireEvent.keyDown(screen.getByRole("radiogroup"), { key: "Enter" }); + expect(onChange).not.toHaveBeenCalled(); + }); + + it("disabled locks clicks and arrow keys and dims the group", () => { + const onChange = vi.fn(); + render(); + const group = screen.getByRole("radiogroup"); + expect(group).toHaveAttribute("aria-disabled", "true"); + expect(group.className).toContain("opacity-40"); + const [simple] = radios(); + expect(simple).toBeDisabled(); + fireEvent.click(simple); + fireEvent.keyDown(group, { key: "ArrowLeft" }); + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx b/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx new file mode 100644 index 00000000..515865fc --- /dev/null +++ b/ornn-web/src/components/skill/generative/GenerationModeToggle.tsx @@ -0,0 +1,112 @@ +/** + * GenerationModeToggle — SIMPLE | ADVANCED segmented control for the + * generative composer row (#1242). + * + * Shares the composer-chip vocabulary of ModelPicker / QuotaInline so + * the three read as one instrument strip: JetBrains Mono micro-label, + * `rounded-sm` hairline frame on `bg-elevated/40`, ember for the + * selected segment only, explicit `focus-visible` ring. A + * `role="radiogroup"` with two `role="radio"` buttons; Left / Right + * arrows move the selection so it is keyboard-operable without a + * pointer. + * + * Stateless — the parent owns the mode (persisted by + * `usePreferredGenerationMode`) and locks the control while a + * generation is streaming. Copy comes from `useGenerationModeCopy` so + * the composer hint row renders the same strings. + * + * @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"; + +export interface GenerationModeToggleProps { + value: GenerationMode; + onChange: (mode: GenerationMode) => void; + /** Locks the control (e.g. while a generation is streaming). */ + disabled?: boolean | undefined; + className?: string | undefined; +} + +export function GenerationModeToggle({ + value, + onChange, + disabled = false, + className = "", +}: 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) => { + if (disabled) return; + if (e.key === "ArrowRight" || e.key === "ArrowDown") { + e.preventDefault(); + move(1); + } else if (e.key === "ArrowLeft" || e.key === "ArrowUp") { + e.preventDefault(); + move(-1); + } + }; + + return ( +
+ + {t("generative.modeLabel", "Mode")} + +
+ {GENERATION_MODES.map((mode) => { + const selected = mode === value; + return ( + + ); + })} +
+
+ ); +} 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/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/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/hooks/useSkillGeneration.test.tsx b/ornn-web/src/hooks/useSkillGeneration.test.tsx new file mode 100644 index 00000000..edcad9bb --- /dev/null +++ b/ornn-web/src/hooks/useSkillGeneration.test.tsx @@ -0,0 +1,222 @@ +/** + * UT-WEB-SKILL-GENERATION-HOOK-001 (#1242) + * + * First tests for the generation lifecycle hook. The SSE client is + * stubbed to capture `(params, onEvent)` so the test can replay server + * frames; the real parser builds the preview. Pins: request params + * (messages transcript, modelId, mode), the phase machine across + * start → tokens → complete / error, multi-turn history accumulation, + * abort / reset, and the preview editing helpers. + * + * @module hooks/useSkillGeneration.test + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import type { GenerationStreamEvent } from "@/types/streaming"; +import type { GenerateStreamParams, StreamHandle } from "@/services/generateStreamApi"; + +let lastParams: GenerateStreamParams | null = null; +let lastOnEvent: ((e: GenerationStreamEvent) => void) | null = null; +const abortSpy = vi.fn(); +let streamCalls = 0; + +vi.mock("@/services/generateStreamApi", () => ({ + generateSkillStream: ( + params: GenerateStreamParams, + onEvent: (e: GenerationStreamEvent) => void, + ): StreamHandle => { + streamCalls += 1; + lastParams = params; + lastOnEvent = onEvent; + return { abort: abortSpy }; + }, +})); + +import { useSkillGeneration } from "./useSkillGeneration"; + +const RAW_SIMPLE = JSON.stringify({ + name: "demo-skill", + description: "A demo skill for the hook tests.", + category: "plain", + tags: ["demo"], + readmeBody: "# Demo\n\nBody.", + runtimes: [], + dependencies: [], + envVars: [], + scripts: [], + references: [], + assets: [], +}); + +const RAW_ADVANCED = JSON.stringify({ + ...JSON.parse(RAW_SIMPLE), + name: "advanced-skill", + scripts: [{ filename: "main.js", content: "console.log(1)" }], + references: [{ filename: "api.md", content: "# API" }], +}); + +/** Replay one SSE event through the captured handler, inside act(). */ +function emit(event: GenerationStreamEvent) { + act(() => { + lastOnEvent?.(event); + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + lastParams = null; + lastOnEvent = null; + streamCalls = 0; + abortSpy.mockReset(); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("useSkillGeneration", () => { + it("starts in the input phase with an empty preview", () => { + const { result } = renderHook(() => useSkillGeneration()); + expect(result.current.phase).toBe("input"); + expect(result.current.chatMessages).toEqual([]); + expect(result.current.metadata).toBeNull(); + }); + + it("sendMessage forwards the transcript, modelId and mode (#1242)", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("build a thing", { modelId: "m-1", mode: "simple" })); + expect(lastParams).toEqual({ + messages: [{ role: "user", content: "build a thing" }], + modelId: "m-1", + mode: "simple", + }); + expect(result.current.phase).toBe("generating"); + // User turn + streaming assistant placeholder. + expect(result.current.chatMessages.map((m) => m.role)).toEqual(["user", "assistant"]); + expect(result.current.chatMessages[1]!.isStreaming).toBe(true); + }); + + it("sendMessage without options sends neither modelId nor mode", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x")); + expect(lastParams?.modelId).toBeUndefined(); + expect(lastParams?.mode).toBeUndefined(); + }); + + it("generation_complete parses the package into the preview and closes the turn", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x", { mode: "simple" })); + emit({ type: "generation_start" }); + emit({ type: "generation_complete", raw: RAW_SIMPLE }); + + expect(result.current.phase).toBe("preview"); + expect(result.current.metadata?.name).toBe("demo-skill"); + expect([...result.current.fileContents.keys()]).toEqual(["SKILL.md"]); + const assistant = result.current.chatMessages[1]!; + expect(assistant.isStreaming).toBe(false); + expect(assistant.skillName).toBe("demo-skill"); + expect(assistant.content).toBe("Generated skill: demo-skill"); + // The model's raw answer is appended to the history for the next turn. + expect(result.current.conversationHistory).toEqual([ + { role: "user", content: "x" }, + { role: "assistant", content: RAW_SIMPLE }, + ]); + }); + + it("advanced output lands scripts/ and references/ in the preview", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x", { mode: "advanced" })); + emit({ type: "generation_complete", raw: RAW_ADVANCED }); + expect([...result.current.fileContents.keys()].sort()).toEqual( + ["SKILL.md", "references/api.md", "scripts/main.js"].sort(), + ); + }); + + it("a refinement turn resends the whole transcript with the current mode", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("first", { mode: "advanced" })); + emit({ type: "generation_complete", raw: RAW_SIMPLE }); + act(() => result.current.sendMessage("now simpler", { mode: "simple" })); + + expect(streamCalls).toBe(2); + expect(lastParams?.mode).toBe("simple"); + expect(lastParams?.messages).toEqual([ + { role: "user", content: "first" }, + { role: "assistant", content: RAW_SIMPLE }, + { role: "user", content: "now simpler" }, + ]); + }); + + it("batches token frames into the streaming assistant message", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x")); + emit({ type: "token", content: "{\"na" }); + emit({ type: "token", content: "me\":" }); + // Nothing visible until the flush timer fires. + expect(result.current.streamingTokens).toBe(""); + act(() => { + vi.advanceTimersByTime(60); + }); + expect(result.current.streamingTokens).toBe("{\"name\":"); + expect(result.current.chatMessages[1]!.content).toBe("{\"name\":"); + }); + + it("error frame moves to the error phase and marks the assistant turn", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x", { mode: "simple" })); + emit({ type: "generation_start" }); + emit({ + type: "validation_error", + message: "Simple mode allows SKILL.md only, but the model emitted: scripts", + retrying: true, + }); + emit({ type: "error", message: "LLM produced advanced materials in simple mode after retry" }); + + expect(result.current.phase).toBe("error"); + expect(result.current.error).toContain("simple mode after retry"); + const assistant = result.current.chatMessages[1]!; + expect(assistant.isStreaming).toBe(false); + expect(assistant.content).toContain("Error:"); + expect(result.current.metadata).toBeNull(); + }); + + it("abort() cancels the stream and finalises the streaming message", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x")); + emit({ type: "token", content: "partial" }); + act(() => result.current.abort()); + expect(abortSpy).toHaveBeenCalledTimes(1); + expect(result.current.chatMessages[1]!.isStreaming).toBe(false); + expect(result.current.chatMessages[1]!.content).toBe("partial"); + }); + + it("reset() returns to the initial state and clears the transcript", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x")); + emit({ type: "generation_complete", raw: RAW_SIMPLE }); + act(() => result.current.reset()); + expect(result.current.phase).toBe("input"); + expect(result.current.chatMessages).toEqual([]); + expect(result.current.conversationHistory).toEqual([]); + expect(result.current.metadata).toBeNull(); + // A fresh send after reset starts a new transcript. + act(() => result.current.sendMessage("again")); + expect(lastParams?.messages).toEqual([{ role: "user", content: "again" }]); + }); + + it("updateFileContent and deleteFile edit the preview in place", () => { + const { result } = renderHook(() => useSkillGeneration()); + act(() => result.current.sendMessage("x")); + emit({ type: "generation_complete", raw: RAW_ADVANCED }); + + act(() => result.current.updateFileContent("scripts/main.js", "console.log(2)")); + expect(result.current.fileContents.get("scripts/main.js")).toBe("console.log(2)"); + + act(() => result.current.deleteFile("references/api.md")); + expect(result.current.fileContents.has("references/api.md")).toBe(false); + const root = result.current.parsedFiles[0]!; + const refs = root.children!.find((n) => n.id === "references"); + expect(refs?.children).toEqual([]); + }); +}); diff --git a/ornn-web/src/hooks/useSkillGeneration.ts b/ornn-web/src/hooks/useSkillGeneration.ts index 10485450..e7991970 100644 --- a/ornn-web/src/hooks/useSkillGeneration.ts +++ b/ornn-web/src/hooks/useSkillGeneration.ts @@ -9,7 +9,7 @@ import { useState, useCallback, useRef, useEffect } from "react"; import { generateSkillStream } from "@/services/generateStreamApi"; import { parseGenerationOutput } from "@/utils/generationParser"; import type { GenerationStreamEvent } from "@/types/streaming"; -import type { GenerationPhase, SkillMetadata } from "@/types/skillPackage"; +import type { GenerationMode, GenerationPhase, SkillMetadata } from "@/types/skillPackage"; import type { FileNode } from "@/components/editor/FileTree"; import { track } from "@/lib/analytics"; @@ -37,11 +37,17 @@ interface GenerationState { error: string | null; } +/** Per-send options — the composer's model picker + mode toggle. */ +export interface SendMessageOptions { + /** Overrides the surface default model — the picker passes it in. */ + modelId?: string | undefined; + /** Package shape for this turn (#1242); omitted → server default. */ + mode?: GenerationMode | undefined; +} + export interface UseSkillGenerationReturn extends GenerationState { - /** Send a message (user prompt) to the generation stream. Optional - * `modelId` overrides the surface default — picker passes the - * caller's preferred model in. */ - sendMessage: (content: string, modelId?: string) => void; + /** Send a message (user prompt) to the generation stream. */ + sendMessage: (content: string, options?: SendMessageOptions) => void; /** Abort current stream */ abort: () => void; /** Reset to input phase */ @@ -263,7 +269,8 @@ export function useSkillGeneration(): UseSkillGenerationReturn { }, [cancelFlush]); const sendMessage = useCallback( - (content: string, modelId?: string) => { + (content: string, options: SendMessageOptions = {}) => { + const { modelId, mode } = options; abort(); tokenBufferRef.current = ""; @@ -274,6 +281,7 @@ export function useSkillGeneration(): UseSkillGenerationReturn { promptLength: content.length, turn: conversationHistoryRef.current.length / 2 + 1, modelId: modelId ?? null, + mode: mode ?? null, }); const userMsgId = crypto.randomUUID(); @@ -310,7 +318,7 @@ export function useSkillGeneration(): UseSkillGenerationReturn { })); const handle = generateSkillStream( - { messages: messagesForApi, modelId }, + { messages: messagesForApi, modelId, mode }, handleEvent, ); abortRef.current = handle.abort; 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 ea376c2a..1e70658b 100644 --- a/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx +++ b/ornn-web/src/pages/skill/CreateSkillGenerativePage.tsx @@ -27,10 +27,14 @@ 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 { 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 { PackageIcon } from "@/components/icons"; +import { useGenerationModeCopy, usePreferredGenerationMode } from "@/hooks/useGenerationMode"; +import { useGenerativeDrawer } from "@/hooks/useGenerativeDrawer"; import { useSkillGeneration } from "@/hooks/useSkillGeneration"; import { useCreateSkill } from "@/hooks/useSkills"; import { useMyQuota } from "@/hooks/useQuota"; @@ -56,39 +60,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(); @@ -111,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, pickedModelId ?? undefined), - [generation, pickedModelId], + (content: string) => + generation.sendMessage(content, { modelId: pickedModelId ?? undefined, mode }), + [generation, pickedModelId, mode], ); const handleStarterClick = useCallback((body: string) => { @@ -186,69 +163,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 +236,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 ─── */
@@ -367,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")}

@@ -389,85 +282,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 +312,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 +324,7 @@ export function CreateSkillGenerativePage() { [§ PACKAGE] - {pinnedOpen && ( + {drawer.pinnedOpen && ( {t("generative.pinned", "Pinned")} @@ -498,19 +333,16 @@ export function CreateSkillGenerativePage() {