fix(skills): guard the skill editor against null columns - #18
Merged
Conversation
Opening an imported skill's detail page blanked the whole page:
TypeError: Cannot read properties of null (reading 'trim')
at SkillEditor.useMemo[payload]
The `useState` initialisers already had `|| ""`. The backfill that runs once
the skill finishes loading did not, so a NULL went straight into state and the
next `name.trim()` threw.
It stayed hidden because manually created skills store empty strings in those
columns. The first source that genuinely stored NULL was the importer — when
SKILL.md's frontmatter omits `display_name`, it is left empty on purpose for
the user to fill — and it surfaced this the day it shipped.
The importer now writes empty strings too, but the guard belongs here
regardless: the editor should not depend on every writer agreeing about how to
spell "not filled".
Test asserts both the initialisers and the backfill carry a fallback — the two
correspond to entering the page directly versus clicking through from the
list, so fixing only one leaves the other path broken. Verified by reverting
each guard in turn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pairs with ragent-service#85.
Opening an imported skill's detail page blanked the whole page:
Cause
SkillEditorassigns those fields in two places, and only one had a fallback:A NULL went straight into state, and the next
name.trim()threw.Why it stayed hidden until now
Manually created skills store empty strings in those columns. The first source that genuinely stored NULL was the importer — when SKILL.md's frontmatter omits
display_name, it is deliberately left empty for the user to fill — so the defect surfaced the day that shipped.neodata-financial-searchimported fine earlier because its frontmatter happens to carry adisplay_name;kie-media-gen's does not.Fixed on both sides
The importer now writes empty strings too (ragent-service#85), but the guard belongs here regardless: the editor should not depend on every writer agreeing how to spell "not filled". And the writer should not add a second spelling. Fixing only one side leaves either the next writer or the next reader exposed.
Test
Asserts both the
useStateinitialisers and the backfill carry a fallback. The two correspond to entering the page directly versus clicking through from the list, so fixing one path leaves the other broken.Source-level assertions rather than a render test — this repo has no jsdom, and the rule ("every setter needs a fallback") is exactly the kind that suits enumerating the setters.
Verified by reverting each guard in turn; both mutations fail the test.
287 passing, tsc non-TS5097 errors unchanged at 112.
🤖 Generated with Claude Code