From 4122f83ceba1cd3b2b9c1f8a8a74ad51acd097a2 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Thu, 20 Aug 2026 12:53:21 -0400 Subject: [PATCH 1/3] feat(persona-registry): let handler agents into the cascade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent driven by its `onEvent` entry has no interactive launch to configure, so `harness`, `model`, and `systemPrompt` — already optional on `PersonaSpec` — are optional here too. Requiring them kept exactly the agents the `agents/` directory was added for out of the registry, reporting a valid deployable persona as malformed. `onEvent` and `cloud` now survive parse and merge. An overlay that tweaks env no longer strips the handler entry that makes its base deployable, so the merged spec is a complete agent rather than a partial one. `harnessSettings` stays required: `PersonaSpec` types it non-optional, and `reasoning`/`timeoutSeconds` have no defensible default to invent on a persona's behalf. `onEvent` is validated as a relative path that cannot escape the agent directory, matching the sidecar rule. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/local-personas.test.ts | 85 +++++++++++++++++++ .../persona-registry/src/local-personas.ts | 84 +++++++++++++++--- 2 files changed, 158 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/local-personas.test.ts b/packages/cli/src/local-personas.test.ts index 115defd3..12b00dcf 100644 --- a/packages/cli/src/local-personas.test.ts +++ b/packages/cli/src/local-personas.test.ts @@ -1034,3 +1034,88 @@ 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/); + }); +}); diff --git a/packages/persona-registry/src/local-personas.ts b/packages/persona-registry/src/local-personas.ts index d15274ca..8b556c30 100644 --- a/packages/persona-registry/src/local-personas.ts +++ b/packages/persona-registry/src/local-personas.ts @@ -64,6 +64,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. */ @@ -493,6 +501,27 @@ function isPlainObject(value: unknown): value is Record { * requirement called out in the schema. Throws on absolute paths, * `..` segments, empty strings, or non-`.md` extensions. */ +/** + * Relative-path guard shared by `onEvent` and, with an added `.md` rule, by + * sidecars. A handler that escaped its agent directory would bundle files the + * persona does not own. + */ +function assertSafeRelativePath(value: unknown, context: string): void { + if (typeof value !== 'string' || !value.trim()) { + throw new Error(`${context} must be a non-empty string`); + } + if ( + value.startsWith('/') || + value.startsWith('\\') || + /^[A-Za-z]:/.test(value) + ) { + throw new Error(`${context} must be a relative path; got absolute "${value}"`); + } + if (value.split(/[\\/]+/).some((segment) => segment === '..')) { + throw new Error(`${context} must not contain ".." segments`); + } +} + function assertSidecarPath(value: unknown, context: string): void { if (typeof value !== 'string' || !value.trim()) { throw new Error(`${context} must be a non-empty string`); @@ -588,6 +617,15 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { `${context}.defaultTier is no longer supported (tiers have been removed)` ); } + if (raw.onEvent !== undefined) { + if (typeof raw.onEvent !== 'string' || !raw.onEvent.trim()) { + throw new Error(`${context}.onEvent must be a non-empty string if provided`); + } + assertSafeRelativePath(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 +669,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, + ...(raw.onEvent !== undefined ? { onEvent: (raw.onEvent as string).trim() } : {}), + ...(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 +859,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 +883,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 +941,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 } : {}), @@ -1110,6 +1166,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 +1248,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 } : {}), From c950a196d4f11c5fe43e67319275aef892b340a3 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Thu, 20 Aug 2026 13:05:17 -0400 Subject: [PATCH 2/3] fix(persona-registry): validate onEvent with persona-kit's own rule Hand-rolling the guard drifted from persona-kit twice over. It validated the raw string and stored a trimmed copy, so `" ../x/agent.ts "` cleared the `..` check as the segment `" .."` and escaped the agent directory once trimmed. And it never checked the handler extension, so `onEvent: "README.md"` counted as a handler and skipped the interactive fields the persona never declared. `parseOnEvent` owns both rules and returns the exact string it validated, so the stored value cannot differ from the one that passed. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/local-personas.test.ts | 36 +++++++++++++++++ .../persona-registry/src/local-personas.ts | 39 +++++-------------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/local-personas.test.ts b/packages/cli/src/local-personas.test.ts index 12b00dcf..50112479 100644 --- a/packages/cli/src/local-personas.test.ts +++ b/packages/cli/src/local-personas.test.ts @@ -1119,3 +1119,39 @@ test('onEvent may not escape the agent directory', () => { assert.match(loaded.warnings[0] ?? '', /onEvent must not contain "\.\." segments/); }); }); + +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); + assert.ok((loaded.warnings[0] ?? '').includes('onEvent'), loaded.warnings[0]); + }); +}); diff --git a/packages/persona-registry/src/local-personas.ts b/packages/persona-registry/src/local-personas.ts index 8b556c30..e191a0df 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'; @@ -501,27 +502,6 @@ function isPlainObject(value: unknown): value is Record { * requirement called out in the schema. Throws on absolute paths, * `..` segments, empty strings, or non-`.md` extensions. */ -/** - * Relative-path guard shared by `onEvent` and, with an added `.md` rule, by - * sidecars. A handler that escaped its agent directory would bundle files the - * persona does not own. - */ -function assertSafeRelativePath(value: unknown, context: string): void { - if (typeof value !== 'string' || !value.trim()) { - throw new Error(`${context} must be a non-empty string`); - } - if ( - value.startsWith('/') || - value.startsWith('\\') || - /^[A-Za-z]:/.test(value) - ) { - throw new Error(`${context} must be a relative path; got absolute "${value}"`); - } - if (value.split(/[\\/]+/).some((segment) => segment === '..')) { - throw new Error(`${context} must not contain ".." segments`); - } -} - function assertSidecarPath(value: unknown, context: string): void { if (typeof value !== 'string' || !value.trim()) { throw new Error(`${context} must be a non-empty string`); @@ -617,12 +597,13 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { `${context}.defaultTier is no longer supported (tiers have been removed)` ); } - if (raw.onEvent !== undefined) { - if (typeof raw.onEvent !== 'string' || !raw.onEvent.trim()) { - throw new Error(`${context}.onEvent must be a non-empty string if provided`); - } - assertSafeRelativePath(raw.onEvent, `${context}.onEvent`); - } + // Delegate to persona-kit rather than re-deriving the rule: it owns the + // containment guard AND the handler-extension check, and it returns the + // exact string it validated, so the stored value can never differ from the + // one that passed. A locally trimmed copy would let " ../x/agent.ts " clear + // a `..` check as the segment " .." and then escape once trimmed. + const onEventValue = + raw.onEvent === undefined ? undefined : parseOnEvent(raw.onEvent, `${context}.onEvent`); if (raw.cloud !== undefined && typeof raw.cloud !== 'boolean') { throw new Error(`${context}.cloud must be a boolean if provided`); } @@ -669,7 +650,7 @@ 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, - ...(raw.onEvent !== undefined ? { onEvent: (raw.onEvent as string).trim() } : {}), + ...(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 } : {}), From 41b059b941492dc44d686519bd5482d5284001b1 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Thu, 20 Aug 2026 13:24:38 -0400 Subject: [PATCH 3/3] fix(persona-registry): normalize onEvent before validating it Order matters in both directions. Validating the raw string and storing a trimmed copy let `" ../x/agent.ts "` clear the `..` check as the segment `" .."` and escape once trimmed. Validating without trimming stored `" ./agent.ts"`, which passes every check and then resolves against a directory named `" ."` at deploy. Trimming before `parseOnEvent` makes the validated value and the stored value the same string. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/local-personas.test.ts | 21 ++++++++++++++++++- .../persona-registry/src/local-personas.ts | 20 ++++++++++++------ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/local-personas.test.ts b/packages/cli/src/local-personas.test.ts index 50112479..a28bb7e3 100644 --- a/packages/cli/src/local-personas.test.ts +++ b/packages/cli/src/local-personas.test.ts @@ -1120,6 +1120,24 @@ test('onEvent may not escape the agent directory', () => { }); }); +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'); @@ -1152,6 +1170,7 @@ test('a padded onEvent cannot smuggle a .. segment past the guard', () => { }); const loaded = loadLocalPersonas({ cwd, homeDir }); assert.equal(loaded.byId.has('digest'), false); - assert.ok((loaded.warnings[0] ?? '').includes('onEvent'), loaded.warnings[0]); + // Trimmed before validation, so the traversal guard sees the real path. + assert.match(loaded.warnings[0] ?? '', /onEvent must not contain "\.\." segments/); }); }); diff --git a/packages/persona-registry/src/local-personas.ts b/packages/persona-registry/src/local-personas.ts index e191a0df..b9399471 100644 --- a/packages/persona-registry/src/local-personas.ts +++ b/packages/persona-registry/src/local-personas.ts @@ -597,13 +597,21 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { `${context}.defaultTier is no longer supported (tiers have been removed)` ); } - // Delegate to persona-kit rather than re-deriving the rule: it owns the - // containment guard AND the handler-extension check, and it returns the - // exact string it validated, so the stored value can never differ from the - // one that passed. A locally trimmed copy would let " ../x/agent.ts " clear - // a `..` check as the segment " .." and then escape once trimmed. + // 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(raw.onEvent, `${context}.onEvent`); + 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`); }