Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions packages/cli/src/local-personas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,3 +1034,143 @@ 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/);
});
});
75 changes: 63 additions & 12 deletions packages/persona-registry/src/local-personas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {
type PersonaTag,
type SidecarMdMode,
parseHarnessSettings,
parseInputs
parseInputs,
parseOnEvent
} from '@agentworkforce/persona-kit';
import { listBuiltInPersonas, personaCatalog } from '@agentworkforce/workload-router';

Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -588,6 +597,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(', ')}`);
Expand Down Expand Up @@ -631,6 +658,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
Expand Down Expand Up @@ -819,22 +848,36 @@ 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;
const systemPrompt =
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`);
Expand Down Expand Up @@ -887,9 +930,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 } : {}),
Expand Down Expand Up @@ -1110,6 +1155,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;
Comment on lines +1158 to +1160

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the handler path's declaring directory

When a personas/digest.json overlay inherits ./agent.ts from agents/digest/persona.json, this retains the string but loses its provenance: loaded.paths points to the winning overlay file, while onEvent is defined relative to the lower file. Any consumer following the PersonaSpec contract and resolving the handler relative to the returned persona path therefore looks for personas/agent.ts rather than agents/digest/agent.ts, so the newly tested env-only overlay no longer identifies a deployable handler.

Useful? React with 👍 / 👎.

const cloud = override.cloud ?? base.cloud;
const harnessSettings: HarnessSettings = parseHarnessSettings({
...base.harnessSettings,
...(override.harnessSettings ?? {})
Expand Down Expand Up @@ -1188,9 +1237,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 } : {}),
Expand Down
Loading