New Plugin: Agent Skills Editing#230
Conversation
Provides SKILL.md editing support: - YAML front-matter validation via LocalInspectionTool - JSON Schema injection for name/description fields - Naming convention checks (kebab-case, directory match) - QuickFixes for naming issues Plugin ID: dev.ghostflyby.agent-skills-editing
Following agentskills.io specification: - SkillFrontmatterStructureInspection: YAML frontmatter structure - SkillNameInspection: name field constraints (length, regex, dir match) - SkillDescriptionInspection: description length bounds - SkillOptionalFieldsInspection: license/compatibility/metadata/allowed-tools Add shared SkillFrontmatterAnalyzer with Markdown PSI (MarkdownFrontMatterHeader) Update JSON Schema to match spec fields: name, description, license, compatibility, metadata, allowed-tools. Add org.jetbrains.plugins.yaml dependency.
…duplicate - Use YAML PSI (YAMLFile/YAMLDocument/YAMLMapping/YAMLKeyValue) via InjectedLanguageManager instead of text-based SnakeYAML parsing - Set language="yaml" for all 4 inspections - Remove TruncateQuickFix (truncation is not a real solution) - Remove unknown-field check (not required by spec) - Merge FixSkillNameQuickFix + MatchDirectoryNameQuickFix into common RenameFieldQuickFix - Move @Suppress(UnstableApiUsage) to only SkillFrontmatterAnalyzer.kt
Remove SkillDescriptionInspection, SkillFrontmatterStructureInspection, SkillOptionalFieldsInspection, SkillMdNamingQuickFix, and SkillFrontmatterAnalyzer. All structural checks are covered by JSON Schema. Consolidate RenameFieldQuickFix into SkillNameInspection. Localize plugin.xml displayName/groupName via Bundle keys. Simplify source tree from 8 files to 4 core files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73182c1d28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| private val NAME_REGEX = Regex("""^[a-z0-9]([a-z0-9-]*[a-z0-9])?$""") |
There was a problem hiding this comment.
Reject consecutive hyphens in skill names
The name validator still accepts values like foo--bar because [a-z0-9-]* allows repeated hyphens, even though the schema/inspection description says skill names must not contain consecutive hyphens. In a SKILL.md with name: foo--bar, both this inspection and the schema pattern let invalid metadata pass, so the validation does not enforce the documented skill naming rule; tighten this regex and the mirrored schema pattern to disallow --.
Useful? React with 👍 / 👎.
Qodana for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
…ace experimental API - Add DirToYamlRef reverse reference so renaming YAML name also renames directory - Split into FixNameQuickFix (auto), RenameDirQuickFix (auto), RenameSkillQuickFix (built-in refactoring) - RenameSkillQuickFix triggers IntelliJ refactoring dialog on YAMLScalar, directory follows via reverse reference - Remove direct MarkdownFrontMatterHeader usage, use elementType comparison instead - Simplify single-invalid cases to report consistency and offer rename
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dea0fa52dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return object : PsiElementVisitor() { | ||
| override fun visitFile(file: PsiFile) { | ||
| if (file.name != "SKILL.md" || file !is MarkdownFile) return | ||
| if (file.firstChild.elementType != MarkdownElementType.platformType(FrontMatterHeaderMarkerProvider.FRONT_MATTER_HEADER)) { |
There was a problem hiding this comment.
Guard empty SKILL.md before checking firstChild
For an empty SKILL.md, file.firstChild is null, so this inspection throws while it is supposed to report the missing frontmatter. That disables the quick fix exactly for a common newly-created skill file; use a null-safe check before comparing the element type.
Useful? React with 👍 / 👎.
| |--- | ||
| | | ||
| |""".trimMargin() | ||
| document.insertString(0, frontmatter) |
There was a problem hiding this comment.
Commit the document after inserting frontmatter
When the quick fix edits the Document, it returns without committing it, leaving PSI out of sync until some later implicit commit. In this repo's PSI/VFS safety rules, document writes must call postponed operations/unblock and commitDocument before returning; otherwise follow-up inspections or tests can still see the old SKILL.md contents immediately after applying this fix.
Useful? React with 👍 / 👎.
…creating new ref - SkillDirRenameSearchExecutor now iterates scalar.references and filters by isReferenceTo(dir) instead of constructing a new YamlToDirRef - Make YamlToDirRef internal for cross-file access
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf04d9d738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| holder.registerProblem( | ||
| kv, SkillMdBundle.message("consistency.mismatch", dirName), | ||
| ProblemHighlightType.WARNING, range, | ||
| FixNameQuickFix(dirName), RenameDirQuickFix(rawName), |
There was a problem hiding this comment.
Avoid offering invalid mismatch quick fixes
When the directory name and YAML name are both invalid but different, this branch still offers FixNameQuickFix(dirName) and RenameDirQuickFix(rawName). For a skill under My Skill with name: bad skill, applying either mismatch quick fix writes or renames to a value that isValidSkillName() has already rejected, so the advertised one-click fix leaves the skill metadata invalid instead of moving it toward a valid kebab-case name.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…nt extensions 1. No longer rely on Reference Provider to provide refactor support 2. Handle goto with custom SkillDirRenameSearchExecutor.kt
7ee812d to
94740ef
Compare
Replace dialog-based rename with inline rename using a PsiNamedElement delegate and custom VariableInplaceRenamer subclass. Key changes: - SkillNameInlineElement: PsiNamedElement wrapper for YAML scalar, maps host-file text range for correct inline widget placement - SkillNameInlineReference: precise PsiReference covering only the scalar value range, avoiding whole-element replacement - SkillNameInlineRenamer: suppresses default text-occurrence search (avoiding RenameChooser), accepts kebab-case identifiers, renames parent directory via VFS on commit - SkillNameRenameHandler: uses performInplaceRename() instead of PsiElementRenameHandler.invoke() — no dialog, no chooser
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 309cc43897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val text = viewProvider.document?.charsSequence ?: return null | ||
| var previousOffset = offset - 1 | ||
| while (previousOffset >= 0 && (previousOffset >= text.length || text[previousOffset].isWhitespace())) { |
There was a problem hiding this comment.
Stop scanning back from non-whitespace carets
When the caret is on the first character of the next YAML key after name, the direct lookup returns null for that key, then this fallback starts at offset - 1 and skips the intervening newline back into the name scalar. That makes rename/goto-declaration actions fire on the skill name from positions like the start of description: instead of the element under the caret; only use this backward scan when the caret itself is on trailing whitespace/EOL after the scalar.
Useful? React with 👍 / 👎.
| return result | ||
| } | ||
|
|
||
| internal fun String.isValidSkillName(): Boolean = isNotEmpty() && SKILL_NAME_REGEX.matches(this) |
There was a problem hiding this comment.
Enforce the 64-character skill-name limit
The schema and inspection text both define skill names as 1–64 characters, but the shared validator has no length check, so rename validation and directory/name consistency checks accept names longer than 64 characters. In particular, a matching directory and name value over 64 characters passes this inspection, and inline rename can create that invalid state even though it is outside the documented Agent Skills naming contract.
Useful? React with 👍 / 👎.
- Inline rename via PsiNamedElement delegate + custom VariableInplaceRenamer - Unified inspection fix decision system (one state, three problems, filtered fixes) - QuickFix ordering via PriorityAction (TOP/HIGH/NORMAL/LOW) - Shared performSkillNameInlineRename() helper - SKILL_NAME_REGEX disallows consecutive hyphens - Bundle-localized quickfix names (quickfix.auto.* / quickfix.manual.*) - Docs and TODO cleanup
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc65ba889c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val editor = FileEditorManager.getInstance(project).selectedTextEditor ?: return | ||
| performSkillNameInlineRename(scalar, editor, project) |
There was a problem hiding this comment.
Open the target SKILL.md editor for manual rename
When this quick fix is applied from the Problems/Inspection Results UI, selectedTextEditor can be null or can point at whatever file is currently active rather than the SKILL.md that owns scalarPtr. In that context performSkillNameInlineRename moves the wrong editor to the SKILL.md offset and starts the template against a mismatched document, so the advertised manual rename either does nothing or edits the wrong file; resolve/open the host file for scalar and pass that editor instead of the globally selected one.
Useful? React with 👍 / 👎.
Replace selectedTextEditor with openTextEditor(OpenFileDescriptor) so the inline rename always opens the correct file even when triggered from the Inspection Results panel.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93784d38bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ProblemKind.INVALID_DIRECTORY -> | ||
| it.decision is AutoRenameDir || it.decision is AutoRenameBoth || it.decision is ManualRename | ||
| ProblemKind.MISMATCH -> | ||
| it.priority <= FixPriority.NORMALIZED_SYNC || |
There was a problem hiding this comment.
Hide one-sided normalization fixes for mismatches
When only one side is invalid-but-normalizable and the other side is different, this condition lets the mismatch warning offer one-sided NORMALIZED_SYNC fixes that do not actually resolve the mismatch. For example, name: Bad Skill under directory other-valid gets a mismatch quick fix to rename the directory to bad-skill, but applying it leaves the YAML value as Bad Skill, so the raw values still differ and the invalid-name warning remains; the symmetric Set name to bad-skill case has the same problem for an invalid directory. This is fresh evidence beyond the earlier invalid-value finding: the normalized value is valid, but the one-sided fix still leaves the inspected mismatch in place.
Useful? React with 👍 / 👎.
MISMATCH should only offer fixes that actually resolve the difference: EXACT_SYNC (match the other side's actual value), AutoRenameBoth (change both to the same normalized value), or ManualRename. One-sided NORMALIZED_SYNC fixes leave the raw values still differing.
No description provided.