-
Notifications
You must be signed in to change notification settings - Fork 0
fix(persona-kit): tags optional + free-form per cloud#553 schema #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d1de7b8
431c403
0e8977c
7088ab7
e386c65
29dcdf6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 parseOverride stores runtime When (Refers to line 492) Was this helpful? React with 👍 or 👎 to provide feedback.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 29dcdf6. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,6 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| CODEX_APPROVAL_POLICIES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| CODEX_SANDBOX_MODES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| HARNESS_VALUES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| PERSONA_TAGS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| SIDECAR_MD_MODES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| type CodexApprovalPolicy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| type CodexSandboxMode, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -423,16 +422,24 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| if (raw.description !== undefined && typeof raw.description !== 'string') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`${context}.description must be a string if provided`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (raw.tags !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!Array.isArray(raw.tags) || raw.tags.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`${context}.tags must be a non-empty array of tags if provided`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let normalizedTags: PersonaTag[] | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (raw.tags !== undefined && raw.tags !== null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!Array.isArray(raw.tags)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`${context}.tags must be an array of strings if provided`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tags = new Set<string>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const [idx, tag] of raw.tags.entries()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!PERSONA_TAGS.includes(tag as PersonaTag)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| `${context}.tags[${idx}] must be one of: ${PERSONA_TAGS.join(', ')}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof tag !== 'string' || !tag.trim()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`${context}.tags[${idx}] must be a non-empty string`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const trimmed = tag.trim(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (trimmed.length > 64) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
429
to
+436
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 local-personas.ts parseOverride stores untrimmed tags while parse.ts parseTags trims them The
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed by 431c403, which lands trim + dedupe + sort + collapse-empty for |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`${context}.tags[${idx}] must be ≤64 characters`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags.add(trimmed); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (tags.size > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| normalizedTags = Array.from(tags).sort() as PersonaTag[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -489,7 +496,7 @@ function parseOverride(value: unknown, context: string): LocalPersonaOverride { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| id: raw.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| extends: raw.extends as string | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| intent: raw.intent as string | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags: raw.tags as PersonaTag[] | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(normalizedTags !== undefined ? { tags: normalizedTags } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: raw.description as string | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| skills: raw.skills as PersonaSpec['skills'] | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -804,7 +811,8 @@ function standaloneSpecFromOverride( | |||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: override.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| intent: override.intent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags: requireStandaloneField(override.tags, `${context}.tags`), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Tags are optional per cloud#553 (denormalized catalog metadata). | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(override.tags ? { tags: override.tags } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: requireStandaloneField(override.description, `${context}.description`), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| skills: override.skills ?? [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(inputs ? { inputs } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1009,10 +1017,11 @@ function mergeOverride( | |||||||||||||||||||||||||||||||||||||||||||||||||||
| const claudeMdMode = override.claudeMdMode ?? base.claudeMdMode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const agentsMdMode = override.agentsMdMode ?? base.agentsMdMode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mergedTags = override.tags ?? base.tags; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: override.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| intent: base.intent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags: override.tags ?? base.tags, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(mergedTags ? { tags: mergedTags } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: override.description ?? base.description, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| skills: override.skills ?? base.skills, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(inputs ? { inputs } : {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.