From 0c0b38e3cb958ecc9da4dd3c53df4811c1e91ff7 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Thu, 15 Oct 2026 08:59:25 +0000 Subject: [PATCH] feat(core): Turn the skills hub on with an environment variable instead of a PostHog flag (no-changelog) (#41143) --- .../api-types/src/agents/skills.schema.ts | 6 - .../@n8n/api-types/src/frontend-settings.ts | 2 +- .../@n8n/config/src/configs/agents.config.ts | 9 + packages/@n8n/config/test/config.test.ts | 10 + packages/cli/src/modules/agents/AGENTS.md | 2 +- .../__tests__/skills-hub-rollout.test.ts | 44 ++--- .../__tests__/test-utils/skills-hub-flag.ts | 13 +- .../agents/agent-skill-refs.service.ts | 2 +- .../cli/src/modules/agents/agents.module.ts | 6 +- .../agents/skills/skills-hub-rollout.ts | 19 +- .../agents/skills/skills-hub-startup-copy.ts | 53 +++++- .../export-agent-package.integration.test.ts | 2 +- .../agents/agent-skills-hub.test.ts | 10 +- .../agents/skills-hub-startup-copy.test.ts | 178 +++++++++++++++++- .../agents/skills.controller.test.ts | 4 +- .../repositories/agent.repository.test.ts | 4 +- .../n8n-packages-export-agents.test.ts | 2 +- 17 files changed, 284 insertions(+), 82 deletions(-) diff --git a/packages/@n8n/api-types/src/agents/skills.schema.ts b/packages/@n8n/api-types/src/agents/skills.schema.ts index 020b16d4a6b..bf7279c7d1b 100644 --- a/packages/@n8n/api-types/src/agents/skills.schema.ts +++ b/packages/@n8n/api-types/src/agents/skills.schema.ts @@ -5,12 +5,6 @@ import type { AgentSkill } from './types'; import { paginationSchema } from '../dto/pagination/pagination.dto'; import { Z } from '../zod-class'; -/** - * PostHog flag for the rollout of the skills hub, read for the instance once at startup. - * Off: agents keep their inline skills and the skills routes answer 404. - */ -export const SKILLS_FLAG = '125_context_skills'; - export const SKILLS_MAX_PAGE_SIZE = 100; /** Optional on purpose: without `take` the caller gets every visible skill. */ diff --git a/packages/@n8n/api-types/src/frontend-settings.ts b/packages/@n8n/api-types/src/frontend-settings.ts index 9dd54b1018a..1585fd393d5 100644 --- a/packages/@n8n/api-types/src/frontend-settings.ts +++ b/packages/@n8n/api-types/src/frontend-settings.ts @@ -463,7 +463,7 @@ export type FrontendModuleSettings = { knowledgeBaseEnabled: boolean; /** Whether the AI Assistant proxy is available to the agents module. */ proxyEnabled: boolean; - /** Whether agents use the skills hub (PostHog flag `125_context_skills`, read at startup). */ + /** Whether agents use the skills hub (`N8N_AGENTS_SKILLS_HUB_ENABLED`, read at startup). */ skillsHubEnabled?: boolean; }; }; diff --git a/packages/@n8n/config/src/configs/agents.config.ts b/packages/@n8n/config/src/configs/agents.config.ts index b2fc2e3f3f4..0a256ec249b 100644 --- a/packages/@n8n/config/src/configs/agents.config.ts +++ b/packages/@n8n/config/src/configs/agents.config.ts @@ -66,6 +66,15 @@ export class AgentsConfig { @Env('N8N_AGENTS_TRACING_RECORD_OUTPUTS') tracingRecordOutputs: boolean = true; + /** + * Whether agents keep their skills in the skills hub instead of inline on the agent. + * Read once when the agents module starts; a change needs a restart. Set the same value + * on every main and worker. With it on, each main copies inline skills into the hub at + * startup, once per agent. + */ + @Env('N8N_AGENTS_SKILLS_HUB_ENABLED') + skillsHubEnabled: boolean = false; + /** * Comma-separated list of agent sub-feature modules to enable. Each entry * gates a specific frontend/runtime capability inside the agents module. diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index ec069b2b9d9..4a36d555673 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -743,6 +743,7 @@ describe('GlobalConfig', () => { tracingEnabled: true, tracingRecordInputs: true, tracingRecordOutputs: true, + skillsHubEnabled: false, modules: [], backgroundTasksEnabled: false, sandboxEnabled: false, @@ -801,6 +802,15 @@ describe('GlobalConfig', () => { expect(config.agents.tracingRecordOutputs).toBe(false); }); + it('should parse N8N_AGENTS_SKILLS_HUB_ENABLED from env variables', () => { + process.env = { + N8N_AGENTS_SKILLS_HUB_ENABLED: 'true', + }; + const config = Container.get(GlobalConfig); + + expect(config.agents.skillsHubEnabled).toBe(true); + }); + it('should parse N8N_AGENTS_AI_SANDBOX_SNAPSHOT from env variables', () => { process.env = { N8N_AGENTS_AI_SANDBOX_SNAPSHOT: 'n8n/agent-knowledge:1.2.3', diff --git a/packages/cli/src/modules/agents/AGENTS.md b/packages/cli/src/modules/agents/AGENTS.md index 2daf326dfe2..c55a943f48c 100644 --- a/packages/cli/src/modules/agents/AGENTS.md +++ b/packages/cli/src/modules/agents/AGENTS.md @@ -9,7 +9,7 @@ Guidance for the agents module (`packages/cli/src/modules/agents`). See the [cli ## Skill refs -- The PostHog flag `125_context_skills` (`SKILLS_FLAG`) selects where skills live. `SkillsHubRollout` reads it for the instance once, when the agents module starts, and keeps the value for the life of the process; a change needs a restart. `AgentSkillRefsService.usesSkillsHub` and the `/skills` routes read `SkillsHubRollout`. Turn it on locally with `N8N_FEATURE_FLAG_OVERRIDES`. There are no double writes: data written with one setting is not visible with the other. +- The environment variable `N8N_AGENTS_SKILLS_HUB_ENABLED` (`AgentsConfig.skillsHubEnabled`, default `false`) selects where skills live. `SkillsHubRollout` reads it once, when the agents module starts, and keeps the value for the life of the process; a change needs a restart. Set the same value on every main and worker. `AgentSkillRefsService.usesSkillsHub` and the `/skills` routes read `SkillsHubRollout`. There are no double writes: data written with one setting is not visible with the other. - Hub on: skills live in the skills hub (`skill`, `skill_version`, `skill_file`). A draft keeps its skill refs in `agent_skill_dependency`, in attach order (`createdAt`). A published version keeps its pins in `agent_version_skill_dependency`, in the same order. - Hub on: a skill that an agent page creates is available to the project, to that agent only (`skill.agentId`), or to the instance (`availableTo` on the create request, default `project`). A duplicate of an agent gets its own copy of each agent-only skill. - Hub off: the refs are the `skills` key of `agents.schema` and `agent_history.schema`, and the bodies are the `agents.skills` and `agent_history.skills` columns, as before the skills hub. A ref with `enabled: false` is hidden, and its body stays in the column. diff --git a/packages/cli/src/modules/agents/__tests__/skills-hub-rollout.test.ts b/packages/cli/src/modules/agents/__tests__/skills-hub-rollout.test.ts index 19f72720758..7b1734adb9b 100644 --- a/packages/cli/src/modules/agents/__tests__/skills-hub-rollout.test.ts +++ b/packages/cli/src/modules/agents/__tests__/skills-hub-rollout.test.ts @@ -1,45 +1,37 @@ -import { SKILLS_FLAG } from '@n8n/api-types'; import type { Logger } from '@n8n/backend-common'; +import { AgentsConfig } from '@n8n/config'; import { mock } from 'vitest-mock-extended'; -import type { PostHogClient } from '@/posthog'; - import { SkillsHubRollout } from '../skills/skills-hub-rollout'; describe('SkillsHubRollout', () => { - const postHog = mock(); + let config: AgentsConfig; let rollout: SkillsHubRollout; beforeEach(() => { - vi.resetAllMocks(); - rollout = new SkillsHubRollout(postHog, mock()); + config = new AgentsConfig(); + rollout = new SkillsHubRollout(config, mock()); }); - it('is off until the flag is read', () => { + it('is off by default', () => { + rollout.init(); + + expect(config.skillsHubEnabled).toBe(false); expect(rollout.isEnabled).toBe(false); }); - it('reads the flag for the instance once and keeps the value', async () => { - postHog.getFeatureFlagForInstance.mockResolvedValue(true); + it('is off until the setting is read', () => { + config.skillsHubEnabled = true; - await rollout.init(); - postHog.getFeatureFlagForInstance.mockResolvedValue(false); + expect(rollout.isEnabled).toBe(false); + }); + + it('reads N8N_AGENTS_SKILLS_HUB_ENABLED once and keeps the value until a restart', () => { + config.skillsHubEnabled = true; + + rollout.init(); + config.skillsHubEnabled = false; expect(rollout.isEnabled).toBe(true); - expect(postHog.getFeatureFlagForInstance).toHaveBeenCalledTimes(1); - expect(postHog.getFeatureFlagForInstance).toHaveBeenCalledWith(SKILLS_FLAG); - expect(postHog.getFeatureFlags).not.toHaveBeenCalled(); - }); - - it.each([ - ['missing', undefined], - ['false', false], - ['a variant string', 'test'], - ])('stays off when the flag is %s', async (_label, value) => { - postHog.getFeatureFlagForInstance.mockResolvedValue(value); - - await rollout.init(); - - expect(rollout.isEnabled).toBe(false); }); }); diff --git a/packages/cli/src/modules/agents/__tests__/test-utils/skills-hub-flag.ts b/packages/cli/src/modules/agents/__tests__/test-utils/skills-hub-flag.ts index b31cf7ae25b..aae29568627 100644 --- a/packages/cli/src/modules/agents/__tests__/test-utils/skills-hub-flag.ts +++ b/packages/cli/src/modules/agents/__tests__/test-utils/skills-hub-flag.ts @@ -1,14 +1,13 @@ +import { AgentsConfig } from '@n8n/config'; import { Container } from '@n8n/di'; -import { PostHogClient } from '@/posthog'; - import { SkillsHubRollout } from '../../skills/skills-hub-rollout'; /** - * Sets the `125_context_skills` flag for the instance and reads it again, as a restart - * does. Tests that need the skills hub on or off call it instead of a config value. + * Sets `N8N_AGENTS_SKILLS_HUB_ENABLED` and reads it again, as a restart does. Tests that + * need the skills hub on or off call it instead of changing the config directly. */ -export async function setSkillsHubFlag(enabled: boolean): Promise { - vi.spyOn(Container.get(PostHogClient), 'getFeatureFlagForInstance').mockResolvedValue(enabled); - await Container.get(SkillsHubRollout).init(); +export function setSkillsHubFlag(enabled: boolean): void { + Container.get(AgentsConfig).skillsHubEnabled = enabled; + Container.get(SkillsHubRollout).init(); } diff --git a/packages/cli/src/modules/agents/agent-skill-refs.service.ts b/packages/cli/src/modules/agents/agent-skill-refs.service.ts index 4b5a1f72777..1fad9144270 100644 --- a/packages/cli/src/modules/agents/agent-skill-refs.service.ts +++ b/packages/cli/src/modules/agents/agent-skill-refs.service.ts @@ -38,7 +38,7 @@ export interface AgentSkillsSnapshot { * version, and for the skill bodies they resolve to. All other code reads and * writes skill refs through this service. * - * With the skills hub on (the `125_context_skills` flag), the refs of a draft are the + * With the skills hub on (`N8N_AGENTS_SKILLS_HUB_ENABLED`), the refs of a draft are the * `agent_skill_dependency` rows and the refs of a published version are the * `agent_version_skill_dependency` pins, in attach order. The skill bodies come * from the skills hub. diff --git a/packages/cli/src/modules/agents/agents.module.ts b/packages/cli/src/modules/agents/agents.module.ts index 6f40e7ad225..8af0ff74e85 100644 --- a/packages/cli/src/modules/agents/agents.module.ts +++ b/packages/cli/src/modules/agents/agents.module.ts @@ -11,10 +11,10 @@ export class AgentsModule implements ModuleInterface { const { SandboxSettingsService } = await import('@/services/sandbox-settings.service.js'); Container.get(SandboxSettingsService).registerCredentialUses(); - // Read the skills hub flag once, before anything reads or writes agent skills. With - // the flag on, each main copies inline agent skills into the hub before agents run. + // Read the skills hub setting once, before anything reads or writes agent skills. With + // it on, each main copies inline agent skills into the hub before agents run. const { SkillsHubRollout } = await import('./skills/skills-hub-rollout.js'); - await Container.get(SkillsHubRollout).init(); + Container.get(SkillsHubRollout).init(); const { SkillsHubStartupCopy } = await import('./skills/skills-hub-startup-copy.js'); await Container.get(SkillsHubStartupCopy).runOnStartup(); diff --git a/packages/cli/src/modules/agents/skills/skills-hub-rollout.ts b/packages/cli/src/modules/agents/skills/skills-hub-rollout.ts index 9b637253e4d..efe4ebe59e8 100644 --- a/packages/cli/src/modules/agents/skills/skills-hub-rollout.ts +++ b/packages/cli/src/modules/agents/skills/skills-hub-rollout.ts @@ -1,27 +1,24 @@ -import { SKILLS_FLAG } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; +import { AgentsConfig } from '@n8n/config'; import { Service } from '@n8n/di'; -import { PostHogClient } from '@/posthog'; - /** - * Whether agents use the skills hub, from the PostHog flag `125_context_skills` for the - * instance. `init` reads the flag once at startup and the value holds for the life of the - * process, so the startup copy and every read agree. A change needs a restart. PostHog - * down or diagnostics off means off, unless `N8N_FEATURE_FLAG_OVERRIDES` turns it on. + * Whether agents use the skills hub, from `N8N_AGENTS_SKILLS_HUB_ENABLED`. `init` reads it + * once at startup and the value holds for the life of the process, so the startup copy and + * every read agree. A change needs a restart. */ @Service() export class SkillsHubRollout { private enabled = false; constructor( - private readonly postHogClient: PostHogClient, + private readonly agentsConfig: AgentsConfig, private readonly logger: Logger, ) {} - async init(): Promise { - this.enabled = (await this.postHogClient.getFeatureFlagForInstance(SKILLS_FLAG)) === true; - this.logger.debug('Read the skills hub rollout flag', { enabled: this.enabled }); + init(): void { + this.enabled = this.agentsConfig.skillsHubEnabled; + this.logger.debug('Read the skills hub setting', { enabled: this.enabled }); } get isEnabled(): boolean { diff --git a/packages/cli/src/modules/agents/skills/skills-hub-startup-copy.ts b/packages/cli/src/modules/agents/skills/skills-hub-startup-copy.ts index f9146819a2a..c6eae9f7f7f 100644 --- a/packages/cli/src/modules/agents/skills/skills-hub-startup-copy.ts +++ b/packages/cli/src/modules/agents/skills/skills-hub-startup-copy.ts @@ -22,6 +22,10 @@ const BATCH_SIZE = 100; const WARN_AGENT_IDS = 20; type CopyCounts = { + /** Agents checked, whether or not they needed a copy. */ + scanned: number; + /** Agents with nothing left to copy. */ + skipped: number; agents: number; skills: number; versions: number; @@ -32,6 +36,9 @@ type CopyCounts = { /** What one agent's copy wrote. */ type AgentCounts = Pick; +/** What one agent's copy wrote, and what it copied from. */ +type AgentCopyResult = AgentCounts & { publishedVersions: number; draft: boolean }; + type AgentWork = { agent: Pick; versions: Array>; @@ -89,14 +96,32 @@ export class SkillsHubStartupCopy { } async copyAll(): Promise { - const counts: CopyCounts = { agents: 0, skills: 0, versions: 0, pins: 0, failed: 0 }; + const counts: CopyCounts = { + scanned: 0, + skipped: 0, + agents: 0, + skills: 0, + versions: 0, + pins: 0, + failed: 0, + }; + // A start line without a summary line shows that the copy stopped halfway. + this.logger.info('Started copying inline agent skills into the skills hub'); + const startedAt = Date.now(); let after: Pick | null = null; for (;;) { const agents = await this.agentRepository.findPageForSkillCopy(after, BATCH_SIZE); for (const agent of agents) { + counts.scanned++; try { const copied = await this.copyAgentWithRetry(agent.id); - if (copied) { + if (!copied) counts.skipped++; + else { + // Logged after the commit, so a rolled-back copy never shows up here. + this.logger.debug('Copied the inline skills of an agent into the skills hub', { + agentId: agent.id, + ...copied, + }); counts.agents++; counts.skills += copied.skills; counts.versions += copied.versions; @@ -108,14 +133,24 @@ export class SkillsHubStartupCopy { counts.failed++; this.logger.error('Failed to copy the skills of an agent into the skills hub', { agentId: agent.id, + errorName: error instanceof Error ? error.name : undefined, error: error instanceof Error ? error.message : String(error), }); + if (error instanceof Error) { + this.logger.debug('Stack of the failed skills hub copy', { + agentId: agent.id, + stack: error.stack, + }); + } } } if (agents.length < BATCH_SIZE) break; after = agents[agents.length - 1]; } - this.logger.info('Copied inline agent skills into the skills hub', counts); + this.logger.info('Copied inline agent skills into the skills hub', { + ...counts, + durationMs: Date.now() - startedAt, + }); return counts; } @@ -152,13 +187,17 @@ export class SkillsHubStartupCopy { * time, so one of them fails on the primary key. A second try picks another id. A * failed try rolls back, so only the counts of the try that commits are kept. */ - private async copyAgentWithRetry(agentId: string): Promise { + private async copyAgentWithRetry(agentId: string): Promise { // Most agents have nothing left to copy after the first start. Check that without // a transaction and a row lock, and check again under the lock. if (!(await this.findWork(agentId, {}))) return null; try { return await this.copyAgent(agentId); - } catch { + } catch (error) { + this.logger.warn('Retrying the skills hub copy of an agent after a failed try', { + agentId, + error: error instanceof Error ? error.message : String(error), + }); return await this.copyAgent(agentId); } } @@ -180,7 +219,7 @@ export class SkillsHubStartupCopy { } /** Copies one agent in one transaction. Returns null when it wrote nothing. */ - private async copyAgent(agentId: string): Promise { + private async copyAgent(agentId: string): Promise { return await this.txRunner.run({}, async (ctx) => { // A draft write to this agent waits until the copy commits. The agent row and the // hub-row check are read under the lock, so the copy never uses stale inline @@ -216,7 +255,7 @@ export class SkillsHubStartupCopy { } await this.skillRepository.replaceDependencies(agent.id, refs, ctx); } - return counts; + return { ...counts, publishedVersions: versions.length, draft: copyDraft }; }); } diff --git a/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts b/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts index 29bfa91b288..c97034ac801 100644 --- a/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts +++ b/packages/cli/src/modules/n8n-packages/__tests__/export-agent-package.integration.test.ts @@ -70,7 +70,7 @@ async function readSourceRecords() { } beforeAll(async () => { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); await testModules.loadModules(['agents']); await testDb.init(); agentRepository = Container.get(AgentRepository); diff --git a/packages/cli/test/integration/agents/agent-skills-hub.test.ts b/packages/cli/test/integration/agents/agent-skills-hub.test.ts index dc0fcfd9afc..ed8b8c38101 100644 --- a/packages/cli/test/integration/agents/agent-skills-hub.test.ts +++ b/packages/cli/test/integration/agents/agent-skills-hub.test.ts @@ -58,7 +58,7 @@ describe('Agent skills in the skills hub', () => { const broadcaster = mockInstance(AgentUpdateBroadcaster); beforeAll(async () => { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); mockInstance(Telemetry); mockInstance(AgentRuntimeCacheService); mockInstance(AgentTaskService).requestReconcile.mockResolvedValue(undefined); @@ -291,19 +291,19 @@ describe('Agent skills in the skills hub', () => { await saveInstructions(skill, 'Newer text.'); const version = published.activeVersion!; - await setSkillsHubFlag(false); + setSkillsHubFlag(false); try { // With the flag off, the legacy reader uses the columns that publish wrote. const { skillRefs: refs, skills } = await skillRefs.readVersion(version, {}); expect(refs).toEqual([{ type: 'skill', id: skill }]); expect(await instructionsOf(skills)).toEqual({ [skill]: 'Published text.' }); } finally { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); } }); it('keeps skills on the agent row when the skills hub is off', async () => { - await setSkillsHubFlag(false); + setSkillsHubFlag(false); try { const agent = await createAgent([]); const context = { user: editor, modifiedBy: 'builder' as const }; @@ -335,7 +335,7 @@ describe('Agent skills in the skills hub', () => { expect(await Container.get(SkillRepository).findByIds([created.id])).toEqual([]); expect(await skillRefs.refsForDraft(row, {})).toEqual([{ type: 'skill', id: created.id }]); } finally { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); } }); diff --git a/packages/cli/test/integration/agents/skills-hub-startup-copy.test.ts b/packages/cli/test/integration/agents/skills-hub-startup-copy.test.ts index 22852637653..7570338765f 100644 --- a/packages/cli/test/integration/agents/skills-hub-startup-copy.test.ts +++ b/packages/cli/test/integration/agents/skills-hub-startup-copy.test.ts @@ -1,7 +1,9 @@ import type { AgentSkill } from '@n8n/api-types'; import { createTeamProject, mockInstance, testDb, testModules } from '@n8n/backend-test-utils'; import { Logger } from '@n8n/backend-common'; -import type { Project } from '@n8n/db'; +import { AgentsConfig } from '@n8n/config'; +import { TransactionRunner, type Project } from '@n8n/db'; +import { DataSource } from '@n8n/typeorm'; import { Container } from '@n8n/di'; import { InstanceSettings } from 'n8n-core'; import { randomUUID } from 'node:crypto'; @@ -11,9 +13,15 @@ import { AgentRuntimeCacheService } from '@/modules/agents/agent-runtime-cache.s import { AgentSkillRefsService } from '@/modules/agents/agent-skill-refs.service'; import { AgentUpdateBroadcaster } from '@/modules/agents/agent-update-broadcaster'; import type { Agent } from '@/modules/agents/entities/agent.entity'; +import { AgentSkillDependency } from '@/modules/agents/entities/agent-skill-dependency.entity'; +import { AgentVersionSkillDependency } from '@/modules/agents/entities/agent-version-skill-dependency.entity'; +import { SkillFile } from '@/modules/agents/entities/skill-file.entity'; +import { SkillVersion } from '@/modules/agents/entities/skill-version.entity'; +import { Skill } from '@/modules/agents/entities/skill.entity'; import { AgentHistoryRepository } from '@/modules/agents/repositories/agent-history.repository'; import { AgentRepository } from '@/modules/agents/repositories/agent.repository'; import { SkillRepository } from '@/modules/agents/repositories/skill.repository'; +import { SkillsHubRollout } from '@/modules/agents/skills/skills-hub-rollout'; import { SkillsHubStartupCopy } from '@/modules/agents/skills/skills-hub-startup-copy'; import { Telemetry } from '@/telemetry'; @@ -48,7 +56,7 @@ describe('SkillsHubStartupCopy', () => { beforeEach(async () => { project = await createTeamProject(); - await setSkillsHubFlag(true); + setSkillsHubFlag(true); }); afterEach(async () => { @@ -169,6 +177,44 @@ describe('SkillsHubStartupCopy', () => { expect(await skills.findAgentSkillVersions(agent.id)).toEqual(before); }); + it('logs the start, each copied agent and a summary with scanned and skipped agents', async () => { + const logger = Container.get(Logger); + const info = vi.spyOn(logger, 'info'); + const debug = vi.spyOn(logger, 'debug'); + const agent = await legacyAgent([{ type: 'skill', id: 'tone' }], { tone: body('Tone') }); + await legacyVersion(agent, [{ type: 'skill', id: 'tone' }], { tone: body('Tone') }); + + await copy.copyAll(); + await copy.copyAll(); + + expect(info).toHaveBeenCalledWith('Started copying inline agent skills into the skills hub'); + const copiedLogs = debug.mock.calls.filter( + ([message]) => message === 'Copied the inline skills of an agent into the skills hub', + ); + expect(copiedLogs).toEqual([ + [ + expect.any(String), + expect.objectContaining({ agentId: agent.id, publishedVersions: 1, draft: true }), + ], + ]); + const summaries = info.mock.calls.filter( + ([message]) => message === 'Copied inline agent skills into the skills hub', + ); + expect(summaries).toEqual([ + [ + expect.any(String), + expect.objectContaining({ + scanned: 1, + skipped: 0, + agents: 1, + durationMs: expect.any(Number), + }), + ], + [expect.any(String), expect.objectContaining({ scanned: 1, skipped: 1, agents: 0 })], + ]); + vi.restoreAllMocks(); + }); + it('does not copy a ref that was switched off', async () => { const agent = await legacyAgent( [ @@ -260,6 +306,122 @@ describe('SkillsHubStartupCopy', () => { ); } + describe('restarts', () => { + /** + * Boots the agents module again on the same database, as a restart of the process + * does: the setting is read into a new rollout and a new startup copy runs. Only the + * database carries over, so nothing in memory can stop a second copy. + */ + async function restart(skillsHubEnabled: boolean) { + const config = Container.get(AgentsConfig); + config.skillsHubEnabled = skillsHubEnabled; + const rollout = new SkillsHubRollout(config, Container.get(Logger)); + rollout.init(); + // The services that read agent skills follow the same setting after the restart. + setSkillsHubFlag(skillsHubEnabled); + const freshCopy = new SkillsHubStartupCopy( + Container.get(Logger), + rollout, + Container.get(InstanceSettings), + agents, + skills, + Container.get(TransactionRunner), + ); + await freshCopy.runOnStartup(); + return freshCopy; + } + + /** Row counts of every table the copy writes. */ + async function hubState() { + const manager = Container.get(DataSource).manager; + return { + skills: await manager.count(Skill), + versions: await manager.count(SkillVersion), + files: await manager.count(SkillFile), + draftRefs: await manager.count(AgentSkillDependency), + pins: await manager.count(AgentVersionSkillDependency), + }; + } + + async function agentWithPublishedSkills() { + const refs: InlineRef[] = [ + { type: 'skill', id: 'tone' }, + { type: 'skill', id: 'pricing' }, + ]; + const agent = await legacyAgent(refs, { + tone: body('Tone', 'Draft tone.'), + pricing: { ...body('Pricing'), references: [{ path: 'references/a.md', content: 'A' }] }, + }); + await legacyVersion(agent, refs, { + tone: body('Tone', 'Published tone.'), + pricing: body('Pricing'), + }); + return agent; + } + + it('copies on the first start and writes nothing on later restarts', async () => { + const agent = await agentWithPublishedSkills(); + + await restart(true); + const afterFirst = await hubState(); + await restart(true); + await restart(true); + + expect(afterFirst).toMatchObject({ skills: 2, draftRefs: 2, pins: 2 }); + expect(await hubState()).toEqual(afterFirst); + expect(instructionsOf((await skillRefs.readDraft(agent, {})).skills)).toEqual({ + tone: 'Draft tone.', + pricing: 'Pricing instructions.', + }); + }); + + it('keeps hub edits and detached skills over a restart', async () => { + const agent = await agentWithPublishedSkills(); + await restart(true); + await editInHub('tone', 'Edited in the hub.'); + await skillRefs.replaceDraftRefs(agent, [{ type: 'skill', id: 'tone' }], {}); + const beforeRestart = await hubState(); + + await restart(true); + + expect(await hubState()).toEqual(beforeRestart); + expect(await skillRefs.refsForDraft(agent, {})).toEqual([{ type: 'skill', id: 'tone' }]); + expect(instructionsOf((await skillRefs.readDraft(agent, {})).skills)).toEqual({ + tone: 'Edited in the hub.', + }); + }); + + it('does not copy again when the setting goes off and back on', async () => { + const agent = await agentWithPublishedSkills(); + await restart(true); + const afterFirst = await hubState(); + + await restart(false); + // With the setting off, agents write their inline columns again. + await agents.update( + { id: agent.id }, + { skills: { tone: body('Tone', 'Inline edit.') } as never }, + ); + await restart(true); + + expect(await hubState()).toEqual(afterFirst); + // The agent has hub rows, so the copy leaves it alone: inline edits are not merged. + expect(instructionsOf((await skillRefs.readDraft(agent, {})).skills)).toEqual({ + tone: 'Draft tone.', + pricing: 'Pricing instructions.', + }); + }); + + it('copies each agent once when two mains start at the same time', async () => { + await agentWithPublishedSkills(); + await agentWithPublishedSkills(); + + await Promise.all([restart(true), restart(true)]); + + expect(await hubState()).toMatchObject({ skills: 4, draftRefs: 4, pins: 4 }); + }); + }); + describe('with the flag off', () => { beforeEach(() => { vi.spyOn(Container.get(Logger), 'warn'); @@ -276,7 +438,7 @@ describe('SkillsHubStartupCopy', () => { it('copies nothing and reads only the draft refs and their content', async () => { await legacyAgent([{ type: 'skill', id: 'tone' }], { tone: body('Tone') }); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); const copyReads = [ vi.spyOn(agents, 'findPageForSkillCopy'), vi.spyOn(skills, 'findUnpinnedVersions'), @@ -298,7 +460,7 @@ describe('SkillsHubStartupCopy', () => { await legacyAgent([{ type: 'skill', id: 'tone' }], { tone: body('Tone') }); await copy.copyAll(); await editInHub('tone', 'Edited in the hub.'); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); const findRefs = vi.spyOn(skills, 'findAgentIdsWithDraftRefs'); const settings = Container.get(InstanceSettings); settings.markAsFollower(); @@ -315,7 +477,7 @@ describe('SkillsHubStartupCopy', () => { it('names no agent when the hub runs the same content as the inline skills', async () => { await legacyAgent([{ type: 'skill', id: 'tone' }], { tone: body('Tone') }); await copy.copyAll(); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); await copy.runOnStartup(); @@ -334,7 +496,7 @@ describe('SkillsHubStartupCopy', () => { await copy.copyAll(); await editInHub('tone', 'Edited in the hub.'); await editInHub('pricing', 'Edited in the hub.'); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); await copy.runOnStartup(); @@ -354,7 +516,7 @@ describe('SkillsHubStartupCopy', () => { { name: 'Added', description: 'd', instructions: 'New.', frontmatter: null, files: [] }, ); await skills.replaceDependencies(agent.id, [{ skillId: 'tone' }, { skillId: 'added' }]); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); await copy.runOnStartup(); @@ -826,7 +988,7 @@ describe('SkillsHubStartupCopy', () => { } await copy.copyAll(); for (let index = 0; index < 22; index++) await editInHub(`s${index}`, 'Edited in the hub.'); - await setSkillsHubFlag(false); + setSkillsHubFlag(false); const warn = vi.spyOn(Container.get(Logger), 'warn'); await copy.runOnStartup(); diff --git a/packages/cli/test/integration/agents/skills.controller.test.ts b/packages/cli/test/integration/agents/skills.controller.test.ts index 82440777bef..c6a237f8df3 100644 --- a/packages/cli/test/integration/agents/skills.controller.test.ts +++ b/packages/cli/test/integration/agents/skills.controller.test.ts @@ -45,11 +45,11 @@ describe('SkillsController', () => { }); beforeEach(async () => { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); }); it('hides the skills API when the skills hub is off', async () => { - await setSkillsHubFlag(false); + setSkillsHubFlag(false); await owner.get('/skills').expect(404); await owner.post('/skills').send({ scope: 'instance', skill: body }).expect(404); diff --git a/packages/cli/test/integration/database/repositories/agent.repository.test.ts b/packages/cli/test/integration/database/repositories/agent.repository.test.ts index f1bfc2f2de4..504993f469c 100644 --- a/packages/cli/test/integration/database/repositories/agent.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/agent.repository.test.ts @@ -83,7 +83,7 @@ describe('AgentRepository', () => { afterEach(async () => { // The rollout keeps the flag it read, so set it back off for the next test. - await setSkillsHubFlag(false); + setSkillsHubFlag(false); vi.restoreAllMocks(); await agentHistoryRepo.delete({}); await agentRepo.delete({}); @@ -221,7 +221,7 @@ describe('AgentRepository', () => { }); it('replaces the complete draft and preserves existing task creation times', async () => { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); const agent = await createAgent(); await Container.get(SkillRepository).createSkill( { diff --git a/packages/cli/test/integration/public-api/n8n-packages-export-agents.test.ts b/packages/cli/test/integration/public-api/n8n-packages-export-agents.test.ts index 4ca7cbc610d..a882f85e830 100644 --- a/packages/cli/test/integration/public-api/n8n-packages-export-agents.test.ts +++ b/packages/cli/test/integration/public-api/n8n-packages-export-agents.test.ts @@ -51,7 +51,7 @@ let workflow: WorkflowEntity; let repository: AgentRepository; beforeAll(async () => { - await setSkillsHubFlag(true); + setSkillsHubFlag(true); repository = Container.get(AgentRepository); Container.get(InstanceSettings).markAsLeader(); });