feat(registry): support env vars, dependencies, author, and links in skill manifest#360
feat(registry): support env vars, dependencies, author, and links in skill manifest#360mahsumaktas wants to merge 2 commits intoopenclaw:mainfrom
Conversation
…skill manifest Closes openclaw#350 Add structured declarations for environment variables, package dependencies, author identity, and project links to the skill registry manifest. These fields can be declared in the clawdis metadata block or as top-level frontmatter keys. Changes: - schema: add EnvVarDeclaration, DependencyDeclaration, SkillLinks types to ClawdisSkillMetadata - parser: extract envVars, dependencies, author, links from both clawdis block and top-level frontmatter (fallback for skills without a clawdis block) - UI: render env vars with required/optional badges and descriptions, dependencies with type/version/links, and project links in the skill detail page install card - security: update evaluator prompt to recognize envVars alongside requires.env and primaryEnv - tests: 7 new test cases covering all declaration formats
|
@mahsumaktas is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
src/components/SkillInstallCard.tsx
Outdated
| {env.required === false ? ( | ||
| <span style={{ color: 'var(--ink-soft)', fontSize: '0.75rem' }}>optional</span> | ||
| ) : ( | ||
| <span style={{ color: 'var(--ink-accent)', fontSize: '0.75rem' }}>required</span> | ||
| )} |
There was a problem hiding this comment.
Undefined required displays as "required"
When a skill author declares an env var as { name: "FOO" } without specifying required, the field is undefined (per the schema's boolean?). The condition env.required === false only catches explicit false, so undefined falls through to the else branch and shows "required". This could misrepresent env vars where the author intentionally omitted the flag (meaning "unspecified"), showing them as mandatory.
Consider treating undefined differently, e.g. showing no badge or a distinct label:
| {env.required === false ? ( | |
| <span style={{ color: 'var(--ink-soft)', fontSize: '0.75rem' }}>optional</span> | |
| ) : ( | |
| <span style={{ color: 'var(--ink-accent)', fontSize: '0.75rem' }}>required</span> | |
| )} | |
| {env.required === false ? ( | |
| <span style={{ color: 'var(--ink-soft)', fontSize: '0.75rem' }}>optional</span> | |
| ) : env.required === true ? ( | |
| <span style={{ color: 'var(--ink-accent)', fontSize: '0.75rem' }}>required</span> | |
| ) : null} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/SkillInstallCard.tsx
Line: 84:88
Comment:
**Undefined `required` displays as "required"**
When a skill author declares an env var as `{ name: "FOO" }` without specifying `required`, the field is `undefined` (per the schema's `boolean?`). The condition `env.required === false` only catches explicit `false`, so `undefined` falls through to the else branch and shows "required". This could misrepresent env vars where the author intentionally omitted the flag (meaning "unspecified"), showing them as mandatory.
Consider treating `undefined` differently, e.g. showing no badge or a distinct label:
```suggestion
{env.required === false ? (
<span style={{ color: 'var(--ink-soft)', fontSize: '0.75rem' }}>optional</span>
) : env.required === true ? (
<span style={{ color: 'var(--ink-accent)', fontSize: '0.75rem' }}>required</span>
) : null}
```
How can I resolve this? If you propose a fix, please make it concise.
src/components/SkillInstallCard.tsx
Outdated
| {envVars.map((env) => ( | ||
| <div key={env.name} style={{ display: 'flex', alignItems: 'baseline', gap: '0.5rem' }}> |
There was a problem hiding this comment.
Possible duplicate React keys in environment variable list
The key prop uses only the variable's identifier, which assumes uniqueness. If a manifest accidentally declares the same identifier twice, React will warn about duplicate keys. The dependencies list at line 107 handles this more robustly by appending the array index to the key. Consider applying the same pattern here for consistency.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/SkillInstallCard.tsx
Line: 81:82
Comment:
**Possible duplicate React keys in environment variable list**
The `key` prop uses only the variable's identifier, which assumes uniqueness. If a manifest accidentally declares the same identifier twice, React will warn about duplicate keys. The dependencies list at line 107 handles this more robustly by appending the array index to the key. Consider applying the same pattern here for consistency.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Summary
Closes #350
Skills that require API keys or external packages can now declare them as structured metadata in the registry manifest, replacing the current situation where these details are buried in SKILL.md prose and the security scanner flags the mismatch.
Changes
Schema (
packages/schema/src/schemas.ts)EnvVarDeclarationSchema:{name, required?, description?}— structured env var declarationsDependencyDeclarationSchema:{name, type, version?, url?, repository?}— package dependency declarations (pip/npm/brew/go/cargo/apt/other)SkillLinksSchema:{homepage?, repository?, documentation?, changelog?}— project linksClawdisSkillMetadataSchema(+author: string?)Parser (
convex/lib/skills.ts)envVars,dependencies,author,linksfrom the clawdis metadata blockenv:,dependencies:,author:,links:) — exactly the format proposed in Support declaring required env vars and dependencies in registry manifest #350envVarsaccepts both structured objects and simple string arrays (strings →{name, required: true})"other"UI (
src/components/SkillInstallCard.tsx)required/optionalbadge and descriptionSecurity (
convex/lib/securityPrompt.ts)envVarsalongsiderequires.envandprimaryEnv, reducing false positives from the mismatch the issue describesTests (
convex/lib/skills.test.ts)7 new test cases:
"other"All 27 tests pass ✅ (20 existing + 7 new)
Frontmatter Examples
In clawdis block (recommended)
Top-level frontmatter (fallback)
Backward Compatibility
requires.env(string array) andprimaryEnvcontinue to work as beforeenvVarsprovides richer metadata (descriptions, required flag) alongside the existing systemclawdisis stored asv.optional(v.any())Greptile Summary
This PR adds structured metadata support for environment variables, dependencies, author info, and project links in skill manifests. It extends the schema with three new declarations (
EnvVarDeclarationSchema,DependencyDeclarationSchema,SkillLinksSchema), adds parsing in the backend with a fallback path for top-level frontmatter keys (when no clawdis block exists), renders the new metadata in theSkillInstallCardUI, and updates the security evaluator prompt to recognizeenvVars.parseFrontmatterLevelDeclarations) means skills with top-levelauthor,env,dependencies, orlinkskeys will now produce a non-undefinedclawdisresult where they previously returnedundefined— downstream code uses optional chaining so this should be saferequiredfield display as "required" by default, which may not match the author's intentConfidence Score: 4/5
src/components/SkillInstallCard.tsxhas two minor UI concerns worth addressing.Last reviewed commit: 7588e3e
(5/5) You can turn off certain types of comments like style here!
Context used:
dashboard- AGENTS.md (source)