diff --git a/open-sse/handlers/chatCore/memorySkillsInjection.ts b/open-sse/handlers/chatCore/memorySkillsInjection.ts index 8a18c945a8..2a43c17cbf 100644 --- a/open-sse/handlers/chatCore/memorySkillsInjection.ts +++ b/open-sse/handlers/chatCore/memorySkillsInjection.ts @@ -2,6 +2,7 @@ import { retrieveMemories } from "@/lib/memory/retrieval"; import { getMemorySettings, DEFAULT_MEMORY_SETTINGS, toMemoryRetrievalConfig } from "@/lib/memory/settings"; import { injectMemory, shouldInjectMemory } from "@/lib/memory/injection"; import { injectSkills } from "@/lib/skills/injection"; +import { skillRegistry } from "@/lib/skills/registry"; import { FORMATS } from "../../translator/formats.ts"; import { detectCachingContext } from "../../services/compression/cachingAware.ts"; @@ -138,6 +139,12 @@ export async function injectMemoryAndSkills({ } if (memoryOwnerId && memorySettings?.skillsEnabled) { + // Ensure the registry cache is warm before listing: on a cold/fresh + // process skills that exist only in the DB would be missed (false + // negative -> silent skip). loadFromDatabase() is a no-op when the cache + // is already warm (TTL = 60 s), so repeated calls are cheap. Mirrors the + // pattern in src/lib/skills/interception.ts (#2815). + await skillRegistry.loadFromDatabase(memoryOwnerId); const existingTools = Array.isArray(body.tools) ? body.tools : []; const mergedTools = injectSkills({ provider: getSkillsProviderForFormat(sourceFormat), diff --git a/skills/ponytail/SKILL.md b/skills/ponytail/SKILL.md new file mode 100644 index 0000000000..b2edd3b7d5 --- /dev/null +++ b/skills/ponytail/SKILL.md @@ -0,0 +1,129 @@ +--- +name: ponytail +description: "External agent skill (MIT, github.com/DietrichGebert/ponytail). Forces the laziest solution that actually works: question whether the task needs to exist at all (YAGNI), reuse what is already in the codebase, reach for the standard library before custom code, native platform features before dependencies, one line before fifty. Climb the ladder on every coding task — writing, adding, refactoring, fixing, reviewing, or designing code, and choosing libraries or dependencies. Supports intensity levels: lite, full (default), ultra. Never cut validation, error handling, security, or accessibility." +--- + + + + +--- +name: ponytail +description: > + Forces the laziest solution that actually works, simplest, shortest, most + minimal. Channels a senior dev who has seen everything: question whether the + task needs to exist at all (YAGNI), reach for the standard library before + custom code, native platform features before dependencies, one line before + fifty. Supports intensity levels: lite, full (default), ultra. Use on ANY + coding task: writing, adding, refactoring, fixing, reviewing, or designing + code, and choosing libraries or dependencies. Also use whenever the user + says "ponytail", "be lazy", "lazy mode", "simplest solution", "minimal + solution", "yagni", "do less", or "shortest path", or complains about + over-engineering, bloat, boilerplate, or unnecessary dependencies. Do NOT + use for non-coding requests (general knowledge, prose, translation, + summaries, recipes). +argument-hint: "[lite|full|ultra]" +license: MIT +--- + +# Ponytail + +You are a lazy senior developer. Lazy means efficient, not careless. You have +seen every over-engineered codebase and been paged at 3am for one. The best +code is the code never written. + +## Persistence + +ACTIVE EVERY RESPONSE. No drift back to over-building. Still active if +unsure. Off only: "stop ponytail" / "normal mode". Default: **full**. +Switch: `/ponytail lite|full|ultra`. + +## The ladder + +Stop at the first rung that holds: + +1. **Does this need to exist at all?** Speculative need = skip it, say so in one line. (YAGNI) +2. **Already in this codebase?** A helper, util, type, or pattern that already lives here → reuse it. Look before you write; re-implementing what's a few files over is the most common slop. +3. **Stdlib does it?** Use it. +4. **Native platform feature covers it?** `` over a picker lib, CSS over JS, DB constraint over app code. +5. **Already-installed dependency solves it?** Use it. Never add a new one for what a few lines can do. +6. **Can it be one line?** One line. +7. **Only then:** the minimum code that works. + +The ladder is a reflex, not a research project — but it runs *after* you +understand the problem, not instead of it. Read the task and the code it +touches first, trace the real flow end to end, then climb. Two rungs work → +take the higher one and move on. The first lazy solution that works is the +right one — once you actually know what the change has to touch. + +**Bug fix = root cause, not symptom.** A report names a symptom. Before you +edit, grep every caller of the function you're about to touch. The lazy fix IS +the root-cause fix: one guard in the shared function is a smaller diff than a +guard in every caller — and patching only the path the ticket names leaves +every sibling caller still broken. Fix it once, where all callers route through. + +## Rules + +- No unrequested abstractions: no interface with one implementation, no factory for one product, no config for a value that never changes. +- No boilerplate, no scaffolding "for later", later can scaffold for itself. +- Deletion over addition. Boring over clever, clever is what someone decodes at 3am. +- Fewest files possible. Shortest working diff wins — but only once you understand the problem. The smallest change in the wrong place isn't lazy, it's a second bug. +- Complex request? Ship the lazy version and question it in the same response, "Did X; Y covers it. Need full X? Say so." Never stall on an answer you can default. +- Two stdlib options, same size? Take the one that's correct on edge cases. Lazy means writing less code, not picking the flimsier algorithm. +- Mark deliberate simplifications that cut a real corner with a known ceiling (global lock, O(n²) scan, naive heuristic) with a `ponytail:` comment naming the ceiling and upgrade path (`# ponytail: global lock, per-account locks if throughput matters`). + +## Output + +Code first. Then at most three short lines: what was skipped, when to add it. +No essays, no feature tours, no design notes. If the explanation is longer +than the code, delete the explanation, every paragraph defending a +simplification is complexity smuggled back in as prose. Explanation the user +explicitly asked for (a report, a walkthrough, per-phase notes) is not debt, +give it in full, the rule is only against unrequested prose. + +Pattern: `[code] → skipped: [X], add when [Y].` + +## Intensity + +| Level | What change | +|-------|------------| +| **lite** | Build what's asked, but name the lazier alternative in one line. User picks. | +| **full** | The ladder enforced. Stdlib and native first. Shortest diff, shortest explanation. Default. | +| **ultra** | YAGNI extremist. Deletion before addition. Ship the one-liner and challenge the rest of the requirement in the same breath. | + +Example: "Add a cache for these API responses." +- lite: "Done, cache added. FYI: `functools.lru_cache` covers this in one line if you'd rather not own a cache class." +- full: "`@lru_cache(maxsize=1000)` on the fetch function. Skipped custom cache class, add when lru_cache measurably falls short." +- ultra: "No cache until a profiler says so. When it does: `@lru_cache`. A hand-rolled TTL cache class is a bug farm with a hit rate." + +## When NOT to be lazy + +Never simplify away: input validation at trust boundaries, error handling +that prevents data loss, security measures, accessibility basics, anything +explicitly requested. User insists on the full version → build it, no +re-arguing. + +Never lazy about understanding the problem. The ladder shortens the +solution, never the reading. Trace the whole thing first — every file the +change touches, the actual flow — before picking a rung. Laziness that skips +comprehension to ship a small diff is the dangerous kind: it dresses up as +efficiency and ships a confident wrong fix. Read fully, then be lazy. + +Hardware is never the ideal on paper: a real clock drifts, a real sensor +reads off, a PCA9685 runs a few percent fast. Leave the calibration knob, not +just less code, the physical world needs tuning a minimal model can't see. + +Lazy code without its check is unfinished. Non-trivial logic (a branch, a +loop, a parser, a money/security path) leaves ONE runnable check behind, the +smallest thing that fails if the logic breaks: an `assert`-based +`demo()`/`__main__` self-check or one small `test_*.py`. No frameworks, no +fixtures, no per-function suites unless asked. Trivial one-liners need no +test, YAGNI applies to tests too. + +## Boundaries + +Ponytail governs what you build, not how you talk (pair with Caveman for +terse prose). "stop ponytail" / "normal mode": revert. Level persists until +changed or session end. + +The shortest path to done is the right path. + diff --git a/src/lib/agentSkills/generator.ts b/src/lib/agentSkills/generator.ts index c1ff093c30..5da9691504 100644 --- a/src/lib/agentSkills/generator.ts +++ b/src/lib/agentSkills/generator.ts @@ -202,7 +202,11 @@ export function buildSkillMarkdown( }; const bodyLines = - skill.category === "api" ? buildApiBody(skill, sources) : buildCliBody(skill, sources); + skill.category === "api" + ? buildApiBody(skill, sources) + : skill.category === "cli" + ? buildCliBody(skill, sources) + : ""; // external: content lives in the custom block below // Re-inject custom block if present in existing content let customBlock = ""; diff --git a/src/lib/agentSkills/types.ts b/src/lib/agentSkills/types.ts index 9f9c3e10a7..7054a10fd4 100644 --- a/src/lib/agentSkills/types.ts +++ b/src/lib/agentSkills/types.ts @@ -1,4 +1,4 @@ -export type SkillCategory = "api" | "cli" | "config"; +export type SkillCategory = "api" | "cli" | "config" | "external"; export type SkillArea = // API areas (22) @@ -49,7 +49,9 @@ export type SkillArea = | "cli-eval" | "cli-plugins-skills" | "cli-setup" - | "cli-skill-collector"; + | "cli-skill-collector" + // External (third-party) skills + | "external"; export interface AgentSkill { id: string; // canonical id (e.g. "omni-providers", "cli-serve") diff --git a/src/lib/db/repositories/sqliteComboRepository.ts b/src/lib/db/repositories/sqliteComboRepository.ts index e7fb36c59b..d8ce71b0fa 100644 --- a/src/lib/db/repositories/sqliteComboRepository.ts +++ b/src/lib/db/repositories/sqliteComboRepository.ts @@ -36,6 +36,20 @@ function withSortOrder(payload: string, sortOrder: number | null): JsonRecord { return parsed; } +function getComboId(value: unknown): string | null { + const row = asRecord(value); + return typeof row.id === "string" && row.id.trim().length > 0 ? row.id : null; +} + +function withRowId(payload: string, row: JsonRecord): JsonRecord { + const parsed = withSortOrder(payload, getSortOrder(row)); + const comboId = getComboId(row); + if (comboId && typeof parsed.id !== "string") { + parsed.id = comboId; + } + return parsed; +} + function getComboNameSet( db: ReturnType, extraNames: string[] = [] @@ -72,7 +86,7 @@ function normalizeStoredCombo( function parseComboRow(row: unknown): JsonRecord | null { const payload = getSerializedData(row); if (!payload) return null; - const parsed = withSortOrder(payload, getSortOrder(row)); + const parsed = withRowId(payload, asRecord(row)); // Merge deduplicated column values back into the record const record = asRecord(row); if (record.context_cache_protection !== undefined && record.context_cache_protection !== null) { @@ -97,7 +111,7 @@ function getNextSortOrder() { export async function getCombos(limit?: number, offset?: number) { const db = getDbInstance(); let sql = - "SELECT data, sort_order, context_cache_protection FROM combos ORDER BY sort_order ASC, name COLLATE NOCASE ASC"; + "SELECT id, data, sort_order, context_cache_protection FROM combos ORDER BY sort_order ASC, name COLLATE NOCASE ASC"; const params: unknown[] = []; if (limit !== undefined) { sql += " LIMIT ? OFFSET ?"; @@ -129,7 +143,7 @@ export function getCombosCount(): number { export async function getComboById(id: string) { const db = getDbInstance(); const row = db - .prepare("SELECT data, sort_order, context_cache_protection FROM combos WHERE id = ?") + .prepare("SELECT id, data, sort_order, context_cache_protection FROM combos WHERE id = ?") .get(id); const combo = parseComboRow(row); if (!combo) return null; @@ -139,7 +153,7 @@ export async function getComboById(id: string) { export async function getComboByName(name: string) { const db = getDbInstance(); const row = db - .prepare("SELECT data, sort_order, context_cache_protection FROM combos WHERE name = ?") + .prepare("SELECT id, data, sort_order, context_cache_protection FROM combos WHERE name = ?") .get(name); const combo = parseComboRow(row); if (!combo) return null; @@ -155,7 +169,7 @@ export async function getComboByNameInsensitive(name: string) { const db = getDbInstance(); const row = db .prepare( - "SELECT data, sort_order, context_cache_protection FROM combos WHERE name = ? COLLATE NOCASE" + "SELECT id, data, sort_order, context_cache_protection FROM combos WHERE name = ? COLLATE NOCASE" ) .get(name); const combo = parseComboRow(row); @@ -198,7 +212,7 @@ export async function createCombo(data: JsonRecord) { export async function updateCombo(id: string, data: JsonRecord): Promise { const db = getDbInstance(); const existing = db - .prepare("SELECT data, sort_order, context_cache_protection FROM combos WHERE id = ?") + .prepare("SELECT id, data, sort_order, context_cache_protection FROM combos WHERE id = ?") .get(id); if (!existing) return null; diff --git a/src/lib/memory/store.ts b/src/lib/memory/store.ts index 70884612e4..8f95b84776 100644 --- a/src/lib/memory/store.ts +++ b/src/lib/memory/store.ts @@ -122,6 +122,17 @@ function safeMarkNeedsReindex(id: string, needs: boolean): void { function scheduleVectorUpsert(id: string, content: string): void { setImmediate(async () => { try { + // The upsert is fire-and-forget and embeddings are slow (potion loads + // lazily). Health-check verification (and user deletes) can remove the + // memory before this callback runs — skip quietly instead of spamming + // "memory not found" warnings every sweep interval. + const db = getDbInstance(); + const exists = db.prepare("SELECT rowid FROM memories WHERE id = ?").get(id); + if (!exists) { + log.debug("memory.vec.upsert.skipped_deleted", { id }); + return; + } + const settings = await getMemorySettings(); const resolution = resolveEmbeddingSource(settings); if (!resolution.source) return; diff --git a/src/lib/skills/executor.ts b/src/lib/skills/executor.ts index 8d7db16810..692716d485 100644 --- a/src/lib/skills/executor.ts +++ b/src/lib/skills/executor.ts @@ -1,5 +1,6 @@ import { skillRegistry } from "./registry"; import { SkillExecution, SkillStatus, SkillHandler } from "./types"; +import { builtinSkills } from "./builtins"; import { getDbInstance } from "../db/core"; import { getSettings } from "../db/settings"; import { randomUUID } from "crypto"; @@ -73,7 +74,19 @@ class SkillExecutor { new Date().toISOString() ); - const handler = this.handlers.get(skill.handler); + let handler = this.handlers.get(skill.handler); + if (!handler) { + // Builtin handlers are registered by instrumentation-node at startup, + // but Next.js may compile this module into multiple chunks (each with + // its own SkillExecutor singleton). Fall back to the builtin registry + // so `POST /api/skills/executions` works regardless of which chunk the + // route is served from. + const builtin = builtinSkills[skill.handler]; + if (builtin) { + this.handlers.set(skill.handler, builtin); + handler = builtin; + } + } if (!handler) { throw new Error(`Handler not found: ${skill.handler}`); } diff --git a/src/lib/skills/injection.ts b/src/lib/skills/injection.ts index f66af58b8f..aa60e13253 100644 --- a/src/lib/skills/injection.ts +++ b/src/lib/skills/injection.ts @@ -25,30 +25,74 @@ interface GeminiTool { parameters: Record; } +// Provider tool/function names must match ^[a-zA-Z0-9_-]+$ (OpenAI, DeepSeek, +// Groq, etc.). Skill identifiers are name@version (and names may contain any +// characters), so encode identifiers that would violate the pattern into a +// reversible base64url form. decodeSkillToolName() must be applied on the way +// back in interception before resolving against the registry. +const SKILL_TOOL_NAME_PREFIX = "omr_skill_"; + +export function encodeSkillToolName(name: string, version: string): string { + const identifier = `${name}@${version}`; + if (/^[a-zA-Z0-9_-]+$/.test(identifier)) { + return identifier; + } + return `${SKILL_TOOL_NAME_PREFIX}${Buffer.from(identifier, "utf8").toString("base64url")}`; +} + +export function decodeSkillToolName(toolName: string): string { + if (!toolName.startsWith(SKILL_TOOL_NAME_PREFIX)) { + return toolName; + } + try { + return Buffer.from(toolName.slice(SKILL_TOOL_NAME_PREFIX.length), "base64url").toString("utf8"); + } catch { + return toolName; + } +} + +// Skills store a flat JSON Schema record ({ "text": { "type": "string" } }), +// but Gemini (function_declarations[].parameters) and Anthropic +// (input_schema) require a full object schema with a properties wrapper. +// Normalize to { "type": "object", "properties": {...} } when the stored +// schema is a bare property map. +function normalizeInputSchema(input: Record): Record { + if (typeof input !== "object" || input === null || Array.isArray(input)) { + return input ?? {}; + } + if (typeof input.type === "string") { + return input; + } + return { + type: "object", + properties: input, + }; +} + function skillToOpenAI(skill: Skill): OpenAITool { return { type: "function", function: { - name: `${skill.name}@${skill.version}`, + name: encodeSkillToolName(skill.name, skill.version), description: skill.description, - parameters: skill.schema.input, + parameters: normalizeInputSchema(skill.schema.input), }, }; } function skillToClaude(skill: Skill): ClaudeTool { return { - name: `${skill.name}@${skill.version}`, + name: encodeSkillToolName(skill.name, skill.version), description: skill.description, - input_schema: skill.schema.input, + input_schema: normalizeInputSchema(skill.schema.input), }; } function skillToGemini(skill: Skill): GeminiTool { return { - name: `${skill.name}@${skill.version}`, + name: encodeSkillToolName(skill.name, skill.version), description: skill.description, - parameters: skill.schema.input, + parameters: normalizeInputSchema(skill.schema.input), }; } diff --git a/src/lib/skills/interception.ts b/src/lib/skills/interception.ts index 79b33a8323..c32745ef4f 100644 --- a/src/lib/skills/interception.ts +++ b/src/lib/skills/interception.ts @@ -1,7 +1,7 @@ import { skillExecutor } from "./executor"; import { skillRegistry } from "./registry"; import { builtinSkills } from "./builtins"; -import { detectProvider } from "./injection"; +import { detectProvider, decodeSkillToolName } from "./injection"; import { OMNIROUTE_WEB_SEARCH_FALLBACK_TOOL_NAME } from "@omniroute/open-sse/services/webSearchFallback.ts"; import { OMNIROUTE_WEB_FETCH_FALLBACK_TOOL_NAME } from "@omniroute/open-sse/services/webFetchInterception.ts"; import { logger } from "../../../open-sse/utils/logger.ts"; @@ -113,9 +113,10 @@ export async function interceptToolCalls( }; } - const [name, version] = call.name.includes("@") - ? call.name.split("@") - : [call.name, "latest"]; + const decodedName = decodeSkillToolName(call.name); + const [name, version] = decodedName.includes("@") + ? decodedName.split("@") + : [decodedName, "latest"]; const skillName = version === "latest" ? name : `${name}@${version}`; @@ -226,7 +227,10 @@ function parseArguments(args: string | Record): Record { refreshCatalog(); const catalog = getCatalog(); - assert.equal(catalog.length, 45, `Expected 45 but got ${catalog.length}`); + assert.equal(catalog.length, 46, `Expected 46 but got ${catalog.length}`); }); test("API_SKILL_IDS has exactly 23 entries", () => { @@ -185,9 +185,9 @@ test("filterCatalog({ area: 'nonexistent' }) returns empty array", () => { assert.equal(skills.length, 0); }); -test("filterCatalog({}) returns full catalog (45 entries)", () => { +test("filterCatalog({}) returns full catalog (46 entries)", () => { const skills = filterCatalog({}); - assert.equal(skills.length, 45); + assert.equal(skills.length, 46); }); // ─── refreshCatalog ───────────────────────────────────────────────────────────