Skip to content

Commit 0d3f7f8

Browse files
khaliqgantRicky Schema Cascadeclaudemiyaontherelay
authored
fix(persona-kit): tags optional + free-form per cloud#553 schema (#114)
* fix(persona-kit): make tags optional + free-form per cloud#553 schema parseTags rejected free-form strings (only accepted intent-enum values) AND required the field to be present + non-empty. Both are wrong per the schema in cloud#553 (`tags text[]` — denorm metadata, not a closed enum). This broke the canonical reference persona (examples/notion-essay-pr) whose tags are ['proactive', 'notion', 'github'] — none of which are in the intent enum. First customer (proactive-agents) hit it deploying today. - parseTags returns undefined when tags is missing/null/empty - accepts any string[] with 1..64-char entries, deduped+sorted - removes the closed-enum constraint - widens PersonaSpec.tags to readonly string[] (optional) - regenerates persona.schema.json with the new shape (tags items: { type: string, minLength: 1, maxLength: 64 }; tags dropped from `required`; PersonaTag enum definition removed) - emit-schema.mjs gains a post-process for tags-items bounds matching the existing tightenWorkspaceServiceAccountName pattern - cli list/show + local-personas accept any string tag; --filter-tag is free-form with built-in tags surfaced as a hint - tests cover all the optional/free-form/validation paths Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(cli): normalize free-form persona tags * test(deploy): satisfy persona fixture typing * test(deploy): fix cloud deployUrl fixture typing * test(deploy): widen cloud fixture override type * fix(deploy/cli): satisfy strict typing on cloud test fixture; harden parseOverride null path - packages/deploy/src/modes/cloud.test.ts: route the PersonaSpec<->{cloud:{deployUrl}} casts through `unknown` at the construct site and at all three deployedUrl(...) call sites so TS no longer rejects the bidirectional conversion (CI typecheck blocker). - packages/cli/src/local-personas.ts: omit `tags` from the parseOverride return object when no tags were provided, instead of returning `tags: undefined`. Combined with the existing `raw.tags === null` guard, this guarantees `LocalPersonaOverride.tags` is either a trimmed/deduped/sorted PersonaTag[] or absent — never null at runtime, never an empty array, never an explicit-undefined key. Addresses Devin BUG_0002. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Ricky Schema Cascade <ricky@agent-relay.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Miya <khaliqgant+miya@gmail.com>
1 parent cb3165d commit 0d3f7f8

9 files changed

Lines changed: 200 additions & 63 deletions

File tree

packages/cli/src/cli.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ Commands:
154154
--filter-harness <harness> only show this harness
155155
(${HARNESS_VALUES.join(' | ')})
156156
--filter-tag <tag> only show personas carrying this tag
157-
(${PERSONA_TAGS.join(' | ')})
157+
(free-form; common built-in tags: ${PERSONA_TAGS.join(' | ')})
158158
--no-display-description hide the DESCRIPTION column
159159
show <persona> Print the fully-resolved spec for a single persona,
160160
including which cascade layer defined it (cwd, user,
@@ -2010,7 +2010,7 @@ interface PersonaListRow {
20102010
harness: string;
20112011
model: string;
20122012
intent: string;
2013-
tags: PersonaTag[];
2013+
tags: readonly PersonaTag[];
20142014
description: string;
20152015
}
20162016

@@ -2023,7 +2023,7 @@ function collectPersonaRows(): PersonaListRow[] {
20232023
harness: spec.harness,
20242024
model: spec.model,
20252025
intent: spec.intent,
2026-
tags: spec.tags,
2026+
tags: spec.tags ?? [],
20272027
description: spec.description
20282028
});
20292029
};
@@ -2146,10 +2146,11 @@ function parseListArgs(args: readonly string[]): {
21462146
filterHarness = v as Harness;
21472147
} else if (arg === '--filter-tag') {
21482148
const v = valueOf(i++, arg);
2149-
if (!(PERSONA_TAGS as readonly string[]).includes(v)) {
2150-
die(`list: invalid --filter-tag "${v}". Must be one of: ${PERSONA_TAGS.join(', ')}`);
2149+
const trimmed = v.trim();
2150+
if (!trimmed) {
2151+
die('list: --filter-tag requires a non-empty value.');
21512152
}
2152-
filterTag = v as PersonaTag;
2153+
filterTag = trimmed as PersonaTag;
21532154
} else if (arg === '--display-description') {
21542155
display.description = true;
21552156
} else if (arg === '--no-display-description') {
@@ -2254,7 +2255,7 @@ function formatPersonaShow(spec: PersonaSpec, source: PersonaSource): string {
22542255
lines.push(`PERSONA ${spec.id}`);
22552256
lines.push(`SOURCE ${source}`);
22562257
lines.push(`INTENT ${spec.intent}`);
2257-
lines.push(`TAGS ${spec.tags.length ? spec.tags.join(', ') : '(none)'}`);
2258+
lines.push(`TAGS ${spec.tags && spec.tags.length ? spec.tags.join(', ') : '(none)'}`);
22582259
lines.push(`DESCRIPTION ${spec.description}`);
22592260

22602261
lines.push('');
@@ -3593,15 +3594,15 @@ export function buildPickCandidates(): PickCandidate[] {
35933594
byId.set(spec.id, {
35943595
id: spec.id,
35953596
intent: spec.intent,
3596-
tags: [...spec.tags],
3597+
tags: spec.tags ? [...spec.tags] : [],
35973598
description: spec.description
35983599
});
35993600
}
36003601
for (const [id, spec] of local.byId.entries()) {
36013602
byId.set(id, {
36023603
id,
36033604
intent: spec.intent,
3604-
tags: [...spec.tags],
3605+
tags: spec.tags ? [...spec.tags] : [],
36053606
description: spec.description
36063607
});
36073608
}

packages/cli/src/local-personas.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
CODEX_APPROVAL_POLICIES,
77
CODEX_SANDBOX_MODES,
88
HARNESS_VALUES,
9-
PERSONA_TAGS,
109
SIDECAR_MD_MODES,
1110
type CodexApprovalPolicy,
1211
type CodexSandboxMode,
@@ -423,16 +422,24 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride {
423422
if (raw.description !== undefined && typeof raw.description !== 'string') {
424423
throw new Error(`${context}.description must be a string if provided`);
425424
}
426-
if (raw.tags !== undefined) {
427-
if (!Array.isArray(raw.tags) || raw.tags.length === 0) {
428-
throw new Error(`${context}.tags must be a non-empty array of tags if provided`);
425+
let normalizedTags: PersonaTag[] | undefined;
426+
if (raw.tags !== undefined && raw.tags !== null) {
427+
if (!Array.isArray(raw.tags)) {
428+
throw new Error(`${context}.tags must be an array of strings if provided`);
429429
}
430+
const tags = new Set<string>();
430431
for (const [idx, tag] of raw.tags.entries()) {
431-
if (!PERSONA_TAGS.includes(tag as PersonaTag)) {
432-
throw new Error(
433-
`${context}.tags[${idx}] must be one of: ${PERSONA_TAGS.join(', ')}`
434-
);
432+
if (typeof tag !== 'string' || !tag.trim()) {
433+
throw new Error(`${context}.tags[${idx}] must be a non-empty string`);
435434
}
435+
const trimmed = tag.trim();
436+
if (trimmed.length > 64) {
437+
throw new Error(`${context}.tags[${idx}] must be ≤64 characters`);
438+
}
439+
tags.add(trimmed);
440+
}
441+
if (tags.size > 0) {
442+
normalizedTags = Array.from(tags).sort() as PersonaTag[];
436443
}
437444
}
438445

@@ -489,7 +496,7 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride {
489496
id: raw.id,
490497
extends: raw.extends as string | undefined,
491498
intent: raw.intent as string | undefined,
492-
tags: raw.tags as PersonaTag[] | undefined,
499+
...(normalizedTags !== undefined ? { tags: normalizedTags } : {}),
493500
description: raw.description as string | undefined,
494501
skills: raw.skills as PersonaSpec['skills'] | undefined,
495502
inputs,
@@ -804,7 +811,8 @@ function standaloneSpecFromOverride(
804811
return {
805812
id: override.id,
806813
intent: override.intent,
807-
tags: requireStandaloneField(override.tags, `${context}.tags`),
814+
// Tags are optional per cloud#553 (denormalized catalog metadata).
815+
...(override.tags ? { tags: override.tags } : {}),
808816
description: requireStandaloneField(override.description, `${context}.description`),
809817
skills: override.skills ?? [],
810818
...(inputs ? { inputs } : {}),
@@ -1009,10 +1017,11 @@ function mergeOverride(
10091017
const claudeMdMode = override.claudeMdMode ?? base.claudeMdMode;
10101018
const agentsMdMode = override.agentsMdMode ?? base.agentsMdMode;
10111019

1020+
const mergedTags = override.tags ?? base.tags;
10121021
return {
10131022
id: override.id,
10141023
intent: base.intent,
1015-
tags: override.tags ?? base.tags,
1024+
...(mergedTags ? { tags: mergedTags } : {}),
10161025
description: override.description ?? base.description,
10171026
skills: override.skills ?? base.skills,
10181027
...(inputs ? { inputs } : {}),

packages/deploy/src/modes/cloud.test.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,13 @@ const ENV_KEYS = [
3131
'WORKFORCE_DEPLOY_RETRY_BACKOFF_MS'
3232
] as const;
3333

34-
function persona(overrides: Record<string, unknown> = {}): PersonaSpec {
34+
function persona(overrides: Partial<PersonaSpec> = {}): PersonaSpec {
3535
return {
3636
id: 'demo',
3737
intent: 'documentation',
38-
tags: ['documentation'],
38+
tags: ['documentation'] as const,
3939
description: 'test persona',
40+
skills: [],
4041
harness: 'codex',
4142
model: 'openai-codex/test',
4243
systemPrompt: 'help',
@@ -45,7 +46,7 @@ function persona(overrides: Record<string, unknown> = {}): PersonaSpec {
4546
schedules: [{ name: 'daily', cron: '0 9 * * *' }],
4647
onEvent: './agent.ts',
4748
...overrides
48-
} as PersonaSpec;
49+
};
4950
}
5051

5152
async function withBundle(): Promise<{ bundle: BundleResult; cleanup: () => Promise<void> }> {
@@ -200,20 +201,21 @@ test('cloud URL precedence is flag env, cloud env, persona deployUrl, then defau
200201
return calls.find((call) => call.url.endsWith('/deployments'))?.url;
201202
}
202203

203-
const personaWithUrl = persona({ cloud: { deployUrl: 'https://persona.example.test/' } as unknown });
204+
const personaWithUrl = persona() as unknown as Omit<PersonaSpec, 'cloud'> & { cloud: { deployUrl: string } };
205+
personaWithUrl.cloud = { deployUrl: 'https://persona.example.test/' };
204206
assert.equal(
205207
await deployedUrl({
206208
WORKFORCE_DEPLOY_CLOUD_URL: 'https://flag.example.test/',
207209
WORKFORCE_CLOUD_URL: 'https://env.example.test/'
208-
}, personaWithUrl),
210+
}, personaWithUrl as unknown as PersonaSpec),
209211
'https://flag.example.test/api/v1/workspaces/ws-test/deployments'
210212
);
211213
assert.equal(
212-
await deployedUrl({ WORKFORCE_CLOUD_URL: 'https://env.example.test/' }, personaWithUrl),
214+
await deployedUrl({ WORKFORCE_CLOUD_URL: 'https://env.example.test/' }, personaWithUrl as unknown as PersonaSpec),
213215
'https://env.example.test/api/v1/workspaces/ws-test/deployments'
214216
);
215217
assert.equal(
216-
await deployedUrl({}, personaWithUrl),
218+
await deployedUrl({}, personaWithUrl as unknown as PersonaSpec),
217219
'https://persona.example.test/api/v1/workspaces/ws-test/deployments'
218220
);
219221
assert.equal(

packages/persona-kit/schemas/persona.schema.json

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
"tags": {
1515
"type": "array",
1616
"items": {
17-
"$ref": "#/definitions/PersonaTag"
17+
"type": "string",
18+
"minLength": 1,
19+
"maxLength": 64
1820
},
19-
"description": "Free-form classification labels (from {@link PERSONA_TAGS } ). Every persona has at least one; a persona may carry multiple tags when it spans concerns (e.g. `['testing', 'implementation']`)."
21+
"description": "Free-form catalog labels mirroring `tags text[]` in cloud#553. Tags are denormalized metadata for catalog filtering — they are NOT a closed enum and do NOT overlap with {@link intent } . Authors are free to label personas with provider names, project codes, or workflow shapes (e.g. `['proactive', 'notion', 'github']`). Optional; omitted, `null`, and empty-array values are treated identically."
2022
},
2123
"description": {
2224
"type": "string"
@@ -130,7 +132,6 @@
130132
"required": [
131133
"id",
132134
"intent",
133-
"tags",
134135
"description",
135136
"skills",
136137
"harness",
@@ -160,20 +161,6 @@
160161
}
161162
]
162163
},
163-
"PersonaTag": {
164-
"type": "string",
165-
"enum": [
166-
"planning",
167-
"implementation",
168-
"review",
169-
"testing",
170-
"debugging",
171-
"documentation",
172-
"release",
173-
"discovery",
174-
"analytics"
175-
]
176-
},
177164
"PersonaSkill": {
178165
"type": "object",
179166
"properties": {

packages/persona-kit/scripts/emit-schema.mjs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,26 @@ function tightenWorkspaceServiceAccountName(node) {
8080
}
8181
tightenWorkspaceServiceAccountName(schema);
8282

83+
// Post-process: tighten the persona `tags[]` items to match parseTags
84+
// (non-empty, ≤64 chars). The TS type widens to `readonly string[]` after
85+
// cloud#553 — the generator emits a bare `{ "type": "string" }` because
86+
// the bounds live in the parser. Without this, the schema accepts empty
87+
// or 1MB tag strings that the parser would then reject. Match the
88+
// `tightenWorkspaceServiceAccountName` post-process pattern above.
89+
const PERSONA_TAG_MAX_LEN = 64;
90+
const personaSpecForTags = schema.definitions?.PersonaSpec;
91+
if (
92+
personaSpecForTags &&
93+
personaSpecForTags.properties?.tags?.type === 'array' &&
94+
personaSpecForTags.properties.tags.items?.type === 'string'
95+
) {
96+
personaSpecForTags.properties.tags.items = {
97+
...personaSpecForTags.properties.tags.items,
98+
minLength: 1,
99+
maxLength: PERSONA_TAG_MAX_LEN
100+
};
101+
}
102+
83103
const serialized = `${JSON.stringify(schema, null, 2)}\n`;
84104
await mkdir(dirname(schemaPath), { recursive: true });
85105

packages/persona-kit/src/constants.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import type { Harness, HarnessSkillTarget } from './types.js';
22

33
export const HARNESS_VALUES = ['opencode', 'codex', 'claude'] as const;
4+
/**
5+
* Suggested persona tags used by built-in personas. Tags are free-form
6+
* (`tags text[]` in cloud#553) — this tuple is kept only as a UX hint for
7+
* built-in catalog filtering. New personas may use any string.
8+
*/
49
export const PERSONA_TAGS = [
510
'planning',
611
'implementation',

packages/persona-kit/src/parse.test.ts

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,70 @@ test('parseHarnessSettings rejects non-boolean dangerouslyBypassApprovalsAndSand
325325
);
326326
});
327327

328-
test('parseTags rejects empty arrays and unknown tags', () => {
329-
assert.throws(() => parseTags([], 'tags'), /must be a non-empty array/);
330-
assert.throws(() => parseTags(['nonsense-tag'], 'tags'), /tags\[0\] must be one of:/);
328+
test('parseTags accepts an array of arbitrary string tags', () => {
329+
// Free-form per cloud#553 `tags text[]` — no closed-enum check.
330+
// Output is deduped and sorted for stable serialization.
331+
assert.deepEqual(
332+
parseTags(['proactive', 'notion', 'github'], 'tags'),
333+
['github', 'notion', 'proactive']
334+
);
335+
});
336+
337+
test('parseTags returns undefined when tags is missing', () => {
338+
assert.equal(parseTags(undefined, 'tags'), undefined);
339+
});
340+
341+
test('parseTags returns undefined when tags is null', () => {
342+
assert.equal(parseTags(null, 'tags'), undefined);
343+
});
344+
345+
test('parseTags returns undefined when tags is an empty array', () => {
346+
// Empty array and "no tags" are semantically identical — both collapse
347+
// to undefined so the field stays omitted from the parsed spec.
348+
assert.equal(parseTags([], 'tags'), undefined);
349+
});
350+
351+
test('parseTags rejects non-string entries', () => {
352+
assert.throws(
353+
() => parseTags(['proactive', 123], 'tags'),
354+
/tags\[1\] must be a string/
355+
);
356+
});
357+
358+
test('parseTags rejects empty / whitespace-only strings', () => {
359+
assert.throws(
360+
() => parseTags(['proactive', ' '], 'tags'),
361+
/tags\[1\] must be a non-empty string/
362+
);
363+
assert.throws(
364+
() => parseTags([''], 'tags'),
365+
/tags\[0\] must be a non-empty string/
366+
);
367+
});
368+
369+
test('parseTags rejects entries over 64 characters', () => {
370+
const tooLong = 'x'.repeat(65);
371+
assert.throws(
372+
() => parseTags(['proactive', tooLong], 'tags'),
373+
/tags\[1\] must be 64 characters/
374+
);
375+
});
376+
377+
test('parseTags rejects non-array input', () => {
378+
assert.throws(
379+
() => parseTags('proactive', 'tags'),
380+
/tags must be an array of strings if provided/
381+
);
382+
});
383+
384+
test('parsePersonaSpec omits tags from the spec when the field is missing', () => {
385+
// Tags are optional; the parsed spec should not synthesize an empty array
386+
// when the input omits the field — the schema treats absence and `[]`
387+
// identically per cloud#553's denormalized `tags text[]`.
388+
const raw = { ...validSpec() } as Record<string, unknown>;
389+
delete raw.tags;
390+
const spec = parsePersonaSpec(raw, 'documentation');
391+
assert.equal(spec.tags, undefined);
331392
});
332393

333394
test('parseSkills returns [] for undefined, validates shape per entry', () => {

0 commit comments

Comments
 (0)