Skip to content

Fix skills.sh.json to match the skills.sh schema #255

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.

Sign up for GitHub

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

Open
reidbaker-agent wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix skills.sh.json to match the skills.sh schema #255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions skills.sh.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://skills.sh/schemas/skills.sh.json",
"groups": [
"$schema": "https://skills.sh/schemas/skills.sh.schema.json",
"groupings": [
{
"title": "UI & Layout",
"description": "Responsive Flutter layouts, constraint debugging, and app localization.",
Expand Down
4 changes: 2 additions & 2 deletions tool/generator/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ The default configuration file is located at `resources/flutter_skills.yaml`. It
When adding, renaming, or deleting a `flutter-*` skill in `skills/`:

1. **Update `README.md`**: Run `dart run skills update-readme` (or update the `Available Skills` table in the root `README.md`) so the table matches the `flutter-*` directories in `skills/`.
2. **Update `skills.sh.json`**: Add, rename, or remove the skill entry under `groups` in the root `skills.sh.json` file ([skills.sh documentation](https://www.skills.sh/docs/customize)).
* Only `flutter-*` skills belong in `groups` (synced `dart-*` skills are canonical in `dart-lang/skills`).
2. **Update `skills.sh.json`**: Add, rename, or remove the skill entry under `groupings` in the root `skills.sh.json` file ([skills.sh documentation](https://www.skills.sh/docs/customize)).
* Only `flutter-*` skills belong in `groupings` (synced `dart-*` skills are canonical in `dart-lang/skills`).
* Because `skills.sh` retains historical skill slugs from install telemetry after a skill is renamed or deleted in Git, `"notGrouped": "bottom"` at the end of `skills.sh.json` automatically demotes any retired skill slugs and ungrouped `dart-*` copies into an `Other skills` section at the bottom of the page.
3. **Verify**: Run `dart test` in `tool/generator` (`test/lint_skills_test.dart` verifies that `skills.sh.json` and `README.md` match the `flutter-*` directories in `skills/`).

Expand Down
22 changes: 17 additions & 5 deletions tool/generator/test/lint_skills_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'package:skills_lint/skills_lint.dart';
import 'package:test/test.dart';

const String _expectedSkillsShSchema =
'https://skills.sh/schemas/skills.sh.json';

@reidbaker reidbaker Oct 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best I can tell this url did not exist.

'https://skills.sh/schemas/skills.sh.schema.json';
const int _maxGroupTitleLength = 30;

void _validateGroupTitle(Object? title) {
Expand Down Expand Up @@ -80,23 +80,35 @@ void main() {
jsonDecode(skillsShFile.readAsStringSync()) as Map<String, Object?>;
expect(skillsShJson[r'$schema'], _expectedSkillsShSchema);

// The skills.sh schema sets additionalProperties: false, so unknown keys
// (such as a misspelled "groups") make skills.sh ignore the file.
expect(
skillsShJson.keys,
everyElement(isIn([r'$schema', 'groupings', 'notGrouped'])),
);

// Setting notGrouped to "bottom" instructs skills.sh to render any skill
// not listed in "groups" (including previously deleted/renamed skill slugs
// retained in skills.sh install telemetry and synced dart-* copies) under
// not listed in "groupings" (including previously deleted/renamed skill
// slugs retained in skills.sh install telemetry and synced dart-* copies)
// under
// its built-in "Other skills" heading at the bottom of the repository page
// (the schema only accepts "top", "bottom", or "hidden", and does not
// (the schema only accepts "top" or "bottom", and does not
// support a custom label).
expect(skillsShJson['notGrouped'], 'bottom');
expect(skillsShJson.keys.last, 'notGrouped');

// 3. Validate each group's title, description, and unique skill
// identifiers, and ensure that all grouped skills exist as flutter-*
// directories in skills/ (without typos or missing skills).
final groups = (skillsShJson['groups'] as List<Object?>)
final groups = (skillsShJson['groupings'] as List<Object?>)
.cast<Map<String, Object?>>();
Comment on lines +103 to 104

@gemini-code-assist gemini-code-assist Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent cryptic TypeErrors when parsing skills.sh.json (e.g., if groupings is missing, null, or not a list of maps), it is safer to explicitly assert the type of groupings and its elements using expect before casting.

Suggested change
final groups = (skillsShJson['groupings'] as List<Object?>)
.cast<Map<String, Object?>>();
expect(skillsShJson['groupings'], isA<List<Object?>>());
final groupingsList = skillsShJson['groupings'] as List<Object?>;
expect(groupingsList, everyElement(isA<Map<String, Object?>>()));
final groups = groupingsList.cast<Map<String, Object?>>();

expect(groups, isNotEmpty);

for (final group in groups) {
expect(
group.keys,
everyElement(isIn(['title', 'description', 'skills'])),
);
Comment on lines +108 to +111

@gemini-code-assist gemini-code-assist Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While we validate that the keys of each group only contain allowed properties, we should also explicitly assert that the required skills property is present and is a list. This provides a clear failure message if a group is missing its skills list, rather than throwing a TypeError later in _collectUniqueGroupedSkills.

Suggested change
expect(
group.keys,
everyElement(isIn(['title', 'description', 'skills'])),
);
expect(
group.keys,
everyElement(isIn(['title', 'description', 'skills'])),
);
expect(group['skills'], isA<List<Object?>>());

_validateGroupTitle(group['title']);
_validateGroupDescription(group['description']);
}
Expand Down
Loading