diff --git a/packages/cli/src/local-personas.test.ts b/packages/cli/src/local-personas.test.ts index 115defd3..04af7d80 100644 --- a/packages/cli/src/local-personas.test.ts +++ b/packages/cli/src/local-personas.test.ts @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, rmSync, utimesSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -1034,3 +1034,205 @@ test('a missing agents/ directory is not an error', () => { assert.deepEqual(loaded.warnings, []); }); }); + +// --- handler agents --------------------------------------------------------- +// An agent driven by its `onEvent` entry has no interactive launch to +// configure, so harness/model/systemPrompt are optional. Requiring them kept +// exactly these agents out of the cascade the agents/ dir was added for. + +test('a handler persona loads without interactive fields', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Weekly digest handler.', + cloud: true, + onEvent: './agent.ts', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.deepEqual(loaded.warnings, []); + const spec = loaded.byId.get('digest'); + assert.ok(spec); + assert.equal(spec.onEvent, './agent.ts'); + assert.equal(spec.cloud, true); + assert.equal(spec.harness, undefined); + assert.equal(spec.model, undefined); + assert.equal(spec.systemPrompt, undefined); + }); +}); + +test('a standalone persona with no handler still requires harness', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'interactive'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'interactive', + intent: 'documentation', + description: 'No handler, so an operator launches it.', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.equal(loaded.byId.has('interactive'), false); + assert.match(loaded.warnings[0] ?? '', /harness is required for standalone personas/); + }); +}); + +test('an overlay tweaking env keeps the handler entry it inherits', () => { + withAgentLayer(({ cwd, homeDir, pwdDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Weekly digest handler.', + cloud: true, + onEvent: './agent.ts', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + writeJson(join(pwdDir, 'digest.json'), { id: 'digest', env: { TONE: 'terse' } }); + + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.deepEqual(loaded.warnings, []); + const spec = loaded.byId.get('digest'); + assert.equal(spec?.onEvent, './agent.ts'); + assert.equal(spec?.cloud, true); + assert.equal(spec?.env?.TONE, 'terse'); + }); +}); + +test('onEvent may not escape the agent directory', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Escapes its directory.', + onEvent: '../../../elsewhere/agent.ts', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.equal(loaded.byId.has('digest'), false); + assert.match(loaded.warnings[0] ?? '', /onEvent must not contain "\.\." segments/); + }); +}); + +test('a padded onEvent is normalized before it is stored', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Padded but legal handler path.', + onEvent: ' ./agent.ts', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.deepEqual(loaded.warnings, []); + // Stored untrimmed this resolves against a directory named " .". + assert.equal(loaded.byId.get('digest')?.onEvent, './agent.ts'); + }); +}); + +test('onEvent must point at a handler source file', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Points at prose, not a handler.', + onEvent: 'README.md', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + // Without the extension rule this would read as a handler and skip the + // interactive fields it never declared. + assert.equal(loaded.byId.has('digest'), false); + assert.match(loaded.warnings[0] ?? '', /must point at a \.ts/); + }); +}); + +test('a padded onEvent cannot smuggle a .. segment past the guard', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'digest'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'digest', + intent: 'documentation', + description: 'Escapes once trimmed.', + onEvent: ' ../outside/agent.ts ', + harnessSettings: { reasoning: 'medium', timeoutSeconds: 600 } + }); + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.equal(loaded.byId.has('digest'), false); + // Trimmed before validation, so the traversal guard sees the real path. + assert.match(loaded.warnings[0] ?? '', /onEvent must not contain "\.\." segments/); + }); +}); + +test('a persona.json older than its authoring source warns but still loads', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'proposal-agent'); + mkdirSync(agentDir, { recursive: true }); + writeJson(join(agentDir, 'persona.json'), { + id: 'proposal-agent', + extends: 'persona-maker', + env: { COMPILED: 'stale' } + }); + writeFileSync(join(agentDir, 'persona.ts'), 'export default {}\n'); + // Backdate the artifact rather than sleeping — same relation, no wall clock. + const past = new Date(Date.now() - 60_000); + utimesSync(join(agentDir, 'persona.json'), past, past); + + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.equal(loaded.warnings.length, 1); + assert.match(loaded.warnings[0] ?? '', /persona\.json is older than persona\.ts/); + assert.match(loaded.warnings[0] ?? '', /agentworkforce persona compile/); + // Still served: a forgotten compile must not read as a missing persona. + assert.equal(loaded.byId.get('proposal-agent')?.env?.COMPILED, 'stale'); + }); +}); + +test('a persona.json newer than its authoring source is silent', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'proposal-agent'); + mkdirSync(agentDir, { recursive: true }); + writeFileSync(join(agentDir, 'persona.ts'), 'export default {}\n'); + writeJson(join(agentDir, 'persona.json'), { + id: 'proposal-agent', + extends: 'persona-maker' + }); + const past = new Date(Date.now() - 60_000); + utimesSync(join(agentDir, 'persona.ts'), past, past); + + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.deepEqual(loaded.warnings, []); + assert.ok(loaded.byId.has('proposal-agent')); + }); +}); + +test('staleness is measured against the newest authoring file, not the first', () => { + withAgentLayer(({ cwd, homeDir, agentsDir }) => { + const agentDir = join(agentsDir, 'proposal-agent'); + mkdirSync(agentDir, { recursive: true }); + // An abandoned persona.ts predates the artifact; the live persona.js is + // newer. Measuring against the .ts alone would call this fresh. + writeFileSync(join(agentDir, 'persona.ts'), 'export default {}\n'); + writeJson(join(agentDir, 'persona.json'), { id: 'proposal-agent', extends: 'persona-maker' }); + writeFileSync(join(agentDir, 'persona.js'), 'export default {}\n'); + + const old = new Date(Date.now() - 120_000); + utimesSync(join(agentDir, 'persona.ts'), old, old); + const mid = new Date(Date.now() - 60_000); + utimesSync(join(agentDir, 'persona.json'), mid, mid); + + const loaded = loadLocalPersonas({ cwd, homeDir }); + assert.equal(loaded.warnings.length, 1); + assert.match(loaded.warnings[0] ?? '', /persona\.json is older than persona\.js/); + }); +}); diff --git a/packages/persona-registry/src/local-personas.ts b/packages/persona-registry/src/local-personas.ts index d15274ca..e7c61910 100644 --- a/packages/persona-registry/src/local-personas.ts +++ b/packages/persona-registry/src/local-personas.ts @@ -21,7 +21,8 @@ import { type PersonaTag, type SidecarMdMode, parseHarnessSettings, - parseInputs + parseInputs, + parseOnEvent } from '@agentworkforce/persona-kit'; import { listBuiltInPersonas, personaCatalog } from '@agentworkforce/workload-router'; @@ -64,6 +65,14 @@ export interface LocalPersonaOverride { permissions?: PersonaPermissions; /** Replaces the inherited systemPrompt when set. */ systemPrompt?: string; + /** + * Handler entry, relative to this file's directory. Its presence marks the + * persona as a cloud agent: the handler drives the run, so the interactive + * fields an operator-launched persona must declare are optional here. + */ + onEvent?: string; + /** Deployable as a managed cloud agent. */ + cloud?: boolean; /** Replaces the inherited harness when set. */ harness?: Harness; /** Replaces the inherited model when set. */ @@ -420,18 +429,38 @@ function readNestedLayerEntries( for (const name of names) { const sourceDir = join(dir, name); const path = join(sourceDir, NESTED_PERSONA_FILENAME); - if (isFile(path)) { - entries.push({ label: `${name}/${NESTED_PERSONA_FILENAME}`, path, sourceDir }); + // Compare against the NEWEST authoring file, not the first one that + // exists: a directory that still carries an old persona.ts after moving to + // persona.js would otherwise be measured against the file nobody edits. + const authoredCandidate = NESTED_PERSONA_SOURCE_FILENAMES.map((file) => ({ + file: join(sourceDir, file), + mtimeMs: fileMtimeMs(join(sourceDir, file)) + })) + .filter((candidate): candidate is { file: string; mtimeMs: number } => candidate.mtimeMs !== undefined) + .sort((a, b) => b.mtimeMs - a.mtimeMs)[0]; + const authored = authoredCandidate?.file; + const compiledAt = fileMtimeMs(path); + + if (compiledAt === undefined) { + if (authored) { + warnings.push( + `[${layer.source}] ${name}: ${basename(authored)} has no compiled ${NESTED_PERSONA_FILENAME}; run \`agentworkforce persona compile ${authored}\` to make it loadable.` + ); + } continue; } - const authored = NESTED_PERSONA_SOURCE_FILENAMES.find((file) => - isFile(join(sourceDir, file)) - ); - if (authored) { + + // A stale artifact is the quieter failure: the persona still loads, so + // nothing looks wrong while the edits sitting in the authoring file are + // simply absent. Say so, and keep serving the compiled spec — dropping it + // would turn a forgotten compile into a missing persona. + const authoredAt = authoredCandidate?.mtimeMs; + if (authored !== undefined && authoredAt !== undefined && authoredAt > compiledAt) { warnings.push( - `[${layer.source}] ${name}: ${authored} has no compiled ${NESTED_PERSONA_FILENAME}; run \`agentworkforce persona compile ${join(sourceDir, authored)}\` to make it loadable.` + `[${layer.source}] ${name}: ${NESTED_PERSONA_FILENAME} is older than ${basename(authored)}, so this persona is loading without the latest edits; re-run \`agentworkforce persona compile ${authored}\`.` ); } + entries.push({ label: `${name}/${NESTED_PERSONA_FILENAME}`, path, sourceDir }); } return entries; } @@ -588,6 +617,24 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { `${context}.defaultTier is no longer supported (tiers have been removed)` ); } + // Normalize first, then delegate to persona-kit, which owns both the + // containment guard and the handler-extension check. Order matters in both + // directions: validating the raw string and storing a trimmed copy lets + // " ../x/agent.ts " clear the `..` check as the segment " .." and escape + // once trimmed, while validating without trimming stores " ./agent.ts", + // which passes every check and then resolves to a directory named " ." + // at deploy. Trimming up front makes the validated and stored value one + // and the same. + const onEventValue = + raw.onEvent === undefined + ? undefined + : parseOnEvent( + typeof raw.onEvent === 'string' ? raw.onEvent.trim() : raw.onEvent, + `${context}.onEvent` + ); + if (raw.cloud !== undefined && typeof raw.cloud !== 'boolean') { + throw new Error(`${context}.cloud must be a boolean if provided`); + } if (raw.harness !== undefined) { if (typeof raw.harness !== 'string' || !HARNESS_VALUES.includes(raw.harness as Harness)) { throw new Error(`${context}.harness must be one of: ${HARNESS_VALUES.join(', ')}`); @@ -631,6 +678,8 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { mount: raw.mount as LocalPersonaOverride['mount'], permissions: raw.permissions as LocalPersonaOverride['permissions'], systemPrompt: raw.systemPrompt as string | undefined, + ...(onEventValue !== undefined ? { onEvent: onEventValue } : {}), + ...(raw.cloud !== undefined ? { cloud: raw.cloud as boolean } : {}), ...(raw.harness !== undefined ? { harness: raw.harness as Harness } : {}), ...(raw.model !== undefined ? { model: raw.model as string } : {}), ...(raw.harnessSettings !== undefined @@ -819,12 +868,23 @@ function standaloneSpecFromOverride( cwd = process.cwd() ): PersonaSpec { const context = `standalone persona "${override.id}"`; - const harness = requireStandaloneField(override.harness, `${context}.harness`); - if (!HARNESS_VALUES.includes(harness)) { + // A handler agent is driven by its `onEvent` entry, not by an operator at a + // prompt, so the fields configuring an interactive launch are optional here. + // Requiring them made agents that ship a handler invisible to the cascade: + // the `agents/` directory added for exactly those agents could not load + // them, and the error read as though the persona were malformed. + const isHandler = typeof override.onEvent === 'string' && override.onEvent.trim() !== ''; + + const harness = isHandler + ? override.harness + : requireStandaloneField(override.harness, `${context}.harness`); + if (harness !== undefined && !HARNESS_VALUES.includes(harness)) { throw new Error(`${context}.harness must be one of: ${HARNESS_VALUES.join(', ')}`); } - const model = requireStandaloneField(override.model, `${context}.model`); - if (typeof model !== 'string' || !model.trim()) { + const model = isHandler + ? override.model + : requireStandaloneField(override.model, `${context}.model`); + if (model !== undefined && (typeof model !== 'string' || !model.trim())) { throw new Error(`${context}.model must be a non-empty string`); } const fallbackSystemPrompt = override.claudeMdContent ?? override.agentsMdContent; @@ -832,9 +892,12 @@ function standaloneSpecFromOverride( typeof override.systemPrompt === 'string' && override.systemPrompt.trim() ? override.systemPrompt : fallbackSystemPrompt; - if (typeof systemPrompt !== 'string' || !systemPrompt.trim()) { + if (!isHandler && (typeof systemPrompt !== 'string' || !systemPrompt.trim())) { throw new Error(`${context}.systemPrompt must be a non-empty string`); } + // `harnessSettings` stays required even for a handler: `PersonaSpec` types it + // non-optional, and `reasoning`/`timeoutSeconds` have no defensible default to + // invent on the persona's behalf. Every shipped handler example declares it. const settingsRaw = override.harnessSettings; if (!settingsRaw || !isPlainObject(settingsRaw)) { throw new Error(`${context}.harnessSettings must be an object`); @@ -887,9 +950,11 @@ function standaloneSpecFromOverride( cwd ), ...(inputs ? { inputs } : {}), - harness, - model, - systemPrompt, + ...(override.onEvent !== undefined ? { onEvent: override.onEvent } : {}), + ...(override.cloud !== undefined ? { cloud: override.cloud } : {}), + ...(harness !== undefined ? { harness } : {}), + ...(model !== undefined ? { model } : {}), + ...(systemPrompt !== undefined ? { systemPrompt } : {}), harnessSettings, ...(env ? { env } : {}), ...(mcpServers ? { mcpServers } : {}), @@ -1012,6 +1077,16 @@ function isFile(path: string): boolean { } } +/** Modification time of a regular file, or undefined if it is not one. */ +function fileMtimeMs(path: string): number | undefined { + try { + const st = statSync(path); + return st.isFile() ? st.mtimeMs : undefined; + } catch { + return undefined; + } +} + /** * Resolve relative local skill sources (`./skills/foo.md`) declared by an * override against the directory of the JSON file that declared them, the @@ -1110,6 +1185,10 @@ function mergeOverride( const harness = override.harness ?? base.harness; const model = override.model ?? base.model; const systemPrompt = override.systemPrompt ?? base.systemPrompt; + // An overlay that only tweaks env must not strip the handler entry that + // makes the base a deployable agent. + const onEvent = override.onEvent ?? base.onEvent; + const cloud = override.cloud ?? base.cloud; const harnessSettings: HarnessSettings = parseHarnessSettings({ ...base.harnessSettings, ...(override.harnessSettings ?? {}) @@ -1188,9 +1267,11 @@ function mergeOverride( description: override.description ?? base.description, skills, ...(inputs ? { inputs } : {}), - harness, - model, - systemPrompt, + ...(onEvent !== undefined ? { onEvent } : {}), + ...(cloud !== undefined ? { cloud } : {}), + ...(harness !== undefined ? { harness } : {}), + ...(model !== undefined ? { model } : {}), + ...(systemPrompt !== undefined ? { systemPrompt } : {}), harnessSettings, ...(env ? { env } : {}), ...(mcpServers ? { mcpServers } : {}),