dart_skills_lint v0.3 prep: paperwork, diagnostics, recipes, RULES.md#143
Merged
Conversation
…ption Bumps version to 0.3.0 in preparation for v0.3 cut. Adds pub.dev topics (agent-skills, linter, static-analysis, cli, validation), an issue_tracker field, and rewrites the description to 50-180 chars so pana scoring stops deducting on the description-too-short check.
…cerpt Replaces the generic 'maximum N characters' message with the actual description length plus a deterministic excerpt showing the characters on either side of the cutoff with a |HERE| marker. No rewrite suggested; the author keeps full editorial control.
Every NameFormatRule diagnostic now spells out that the violation is in the SKILL.md frontmatter `name:` field, quotes the offending value, and suggests a corrected form derived by a deterministic normalizer (lowercase, hyphenize, collapse, trim, truncate). The directory-mismatch error explicitly offers both fix directions (edit the field OR rename the dir) instead of silently picking one.
…cs link When a relative link in SKILL.md misses, the error now reports both the literal path as written AND the resolved absolute path, scans the parent directory of the target for the nearest existing filename by Levenshtein distance (threshold = basename.length / 3, clamped >= 1), and surfaces a 'Did you mean ...' suggestion when one is close enough. Absolute-path errors gain a one-line rationale and the same spec link so authors don't have to guess why a hard-coded path is rejected.
…ated Aligns flag semantics with the ecosystem convention used by prettier, eslint, and ruff: `--fix` writes the fixes to disk, and the new `--dry-run` companion flag swaps it back to preview mode. The legacy `--fix-apply` flag still works (so existing pipelines don't break) but is now hidden from --help and emits a deprecation notice to stderr on use. README updated to match.
When the linter is invoked with no flags and neither .claude/skills nor .agents/skills exists, replace the terse 'Missing skills directory' error with a welcoming guide that tells a brand-new user three concrete ways to get started (single skill, root dir, default discovery paths), points to the spec, and reminds them about --help. Still exits 64 so CI pipelines that depend on the failure mode don't silently start passing.
Adds two reference skill directories and a README that tells a new user exactly what each fixture is and how to run the linter against it. The invalid fixture deliberately trips three distinct rules so the README can show what real diagnostic output looks like (and so T3's drift test has something concrete to assert against). pana also rewards a top-level example/ folder.
New test/example_fixtures_test.dart runs the CLI against both fixtures and asserts they behave as the README claims: valid exits 0, invalid exits 1 on invalid-skill-name under defaults, and surfaces disallowed-field + check-absolute-paths once the lower-severity rules are escalated. While writing the test I noticed the invalid fixture's README was overclaiming — only one of the three rules fires under defaults — so the SKILL.md and example/README.md now spell out which rules need a flag to surface as errors. Fixtures + diagnostics now move in lockstep.
…hook Adds two drop-in integration snippets so adopters don't have to invent their own wiring. Recipe 1 is a workflow.yml that runs the linter on every push and PR via dart-lang/setup-dart and pub global activate. Recipe 2 is a self-contained pre-commit hook that uses the same global install — no Husky, no Python pre-commit framework. Both snippets reference the new --fix semantics from DT2; the table-of-contents is extended to expose the new section.
New test/recipe_drift_test.dart extracts every fenced code block under the README's ## Recipes heading and exercises both shipped recipes without going through pub.dev. For the GitHub Actions YAML it parses the document, asserts the expected wiring (dart-lang/setup-dart, pub global activate, --skills-directory invocation), then re-runs each 'dart pub global run dart_skills_lint ...' step against the local example/ fixtures through bin/cli.dart. For the pre-commit hook it extracts the HEREDOC body, rewires the linter call to bin/cli.dart, and runs the hook against both fixtures. If a flag in the recipes ever goes stale, this test now catches it before adopters do. Skipped on Windows; the hook uses POSIX shell.
Adds a Support section explaining where to file bugs (GitHub issues with --help output and a minimal repro), where to ask questions and float ideas (GitHub Discussions, with a fallback instruction in case it's been temporarily disabled), and how to report a security issue privately. Discussions needs to be enabled by a repo admin via the GitHub UI; that's not something a commit can do, so the section says how to recover if Discussions is offline. Table of contents updated.
Spells out exactly what kind of rule change requires what version bump, so adopters can safely set ^1.0.0 in pubspec.yaml without fearing that a pub upgrade turns green CI red. Patch = bug fixes + message rewordings + narrower matches. Minor = new rules (must default to disabled) + perf + diagnostic expansions. Major = any change that can fail a previously-passing skill (default-severity upgrades, broader matches, rule removals, renames). Also tells new-rule authors they must add a RULES.md entry — the contract that T4 will land.
…ection Captures every shipped change in 0.3.0 grouped by surface (Package metadata, CLI, Rules, Documentation, CI). The new ### Rules subsection is the convention going forward — rule-shape changes always land in their own section so adopters can scan the upgrade impact in one glance. A '1.0.0 — planned' placeholder anchors the burn-in promise and explicitly states no rule-shape changes are planned between 0.3.0 and 1.0.0, which is the whole point of the freeze window.
New pana_score job in dart_skills_lint_workflow.yaml installs pana, runs it against the package, parses grantedPoints out of the JSON report, and fails the job if the score drops below 150. pana itself exits 0 on score regressions, so the gating has to be done manually in the workflow. Current local score is 160/160 (was 140/160 before T1 fixed the description), so the 150 floor leaves a small cushion without making future tightening hard. The full pana report is uploaded as an artifact for postmortem on failures.
New publish_dry_run job that runs 'dart pub publish --dry-run' so maintainers can rehearse the v1.0.0 publish flow end-to-end before the real release. Gated behind workflow_dispatch and an explicit job-level if: so it never runs on routine push/PR/schedule events — the dry-run output is noisy and only useful when someone's actively staging a release. Pair with the existing pana_score gate to catch the two most common publish-time failures (metadata / score) before they reach pub.dev.
Runs 'dart fix --apply' and 'dart format' against the files touched by T1-T11 and DT1-DT6 so 'dart analyze --fatal-infos' goes clean. No behavior changes — just type-annotation/control-body cleanup the analyzer was flagging on the recipe drift test, the Levenshtein implementation, and the updated entry_point fix-semantics block.
Adds RULES.md as the authoritative reference for every rule in RuleRegistry. Each rule gets a section with default severity, fixable yes/no, exact diagnostic shape, auto-fix behavior, and the negated CLI flag for disabling. Severity vocabulary and the three configuration surfaces (CLI flag, top-level YAML, per-directory YAML) are documented once at the top. T5 will land next and pin every entry in this file to the corresponding registry entry so the two cannot drift apart. Format chosen: one ## <rule-name> section per rule with sub-bullets, not a summary table. The format gives each rule enough room to spell out the diagnostic shape and auto-fix behavior — the things adopters actually need when they hit an error message in CI.
New test/rules_md_consistency_test.dart parses every '## <rule-name>' section in RULES.md, extracts the documented default severity and fixable flag, and asserts four invariants against RuleRegistry: 1. Every registered rule has a RULES.md entry (catches missing docs). 2. Every RULES.md entry maps to a registered rule (catches stale docs after a rule is removed or renamed). 3. The documented 'Default severity:' value equals the rule's CheckType.defaultSeverity (catches silent severity changes that should have been a major version bump per CONTRIBUTING.md). 4. The documented 'Fixable:' value matches whether the rule's class actually implements FixableRule (catches docs promising a fix that isn't there, and vice versa). Each failure prints exactly which rule and which field diverged, so the fix path is obvious. Dart's RegExp doesn't support \Z, so the parser appends a sentinel '## __end__' heading to terminate the last section cleanly rather than juggling end-of-string anchors.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates dart_skills_lint to version 0.3.0, focusing on CLI polish, enhanced diagnostics, and comprehensive documentation. Key functional changes include updating the --fix flag to apply changes by default, introducing a --dry-run mode, and adding a detailed onboarding guide for new users. Rule diagnostics have been significantly improved, notably adding visual excerpts for length violations and Levenshtein-based suggestions for broken relative paths. The PR also introduces a formal Rule-stability policy, a detailed RULES.md reference, and several "drift-guard" tests to maintain consistency between code and documentation. Reviewer feedback suggested refining the relative path suggestion logic to preserve directory structures and ensure cross-platform path compatibility, as well as improving the robustness of directory listing operations.
Conflicts: - pubspec.yaml: keep the 50-180 char description from this branch (the whole point of the bump was the pana score; main's terse description drops it back to 140/160). - CHANGELOG.md: merge main's 0.3.0 bullets (ConfigParser.loadConfig API, tilde expansion, CLI-vs-test docs) into the structured sections from this branch. Tilde expansion + CLI-vs-test get their own ### API and ### Documentation entries. - entry_point.dart: keep main's refactor that moved defaults discovery into _getEffectiveSkillDirPaths and made it throw MissingDefaultsException, but route the catch back through the champion-tier first-run guide (firstRunGuideMsg) instead of the terse 'Missing skills directory. Checked defaults: ...' fallback main had. Post-merge: dart analyze --fatal-infos clean, dart format clean, 139/139 tests pass (was 126; main added 13), pana 160/160.
reidbaker
commented
May 22, 2026
Contributor
Author
|
Additional comments:
|
Promotes the markdown-link regex onto SkillContext alongside the existing skillStartRegex so the rule files have one source of truth instead of two near-identical copies. Both path rules now use SkillContext.skillStartRegex + SkillContext.markdownLinkRegex. Also drops the agentskills.io/specification#content reference from both diagnostics — that anchor 404s and the absolute/relative path behaviors are best practice, not spec. The portability rationale in the absolute-path message stays.
Moves the Levenshtein distance helper out of relative_paths_rule.dart into lib/src/levenshtein.dart, exported as a top-level public function. Same algorithm, but the triple-nested ternary that picked the minimum of del/ins/sub is now math.min(math.min(del, ins), sub). That clears the dart_code_linter avoid-nested-conditional-expressions finding that was failing Windows CI.
findSiblingSuggestion now takes both the original link text and the resolved path, so it can return the full suggested link (dir prefix joined to the matched basename, forward-slash normalized) instead of just a basename. A subdirectory link like docs/DEATILS.md now suggests docs/DETAILS.md instead of just DETAILS.md, which the user can paste in directly. The candidate set also drops directories (a link almost never points at a dir; suggesting one would mislead) and wraps the listSync() call in a FileSystemException catch so a permission error on the parent dir degrades to 'no suggestion' instead of crashing the run. Promoted to @VisibleForTesting so a follow-up commit can land unit tests for it directly.
Covers the five cases the algorithm needs to get right: no-dir-prefix link, subdir link, no near-miss in the parent dir, missing parent dir, and the directories-aren't-candidates rule. Lives next to relative_paths_test.dart so a regression points at the algorithm rather than the rule plumbing.
Spells out what the helper does at the call site. Pure rename, no behavior change.
New buildLengthDiagnostic helper in lib/src/cutoff_excerpt.dart owns
the 'field is N characters; maximum is M. Cutoff at character M:
...|HERE|...' message shape. description-too-long and the
compatibility check in valid-yaml-metadata both call it, so an author
who hits either limit gets the same kind of pointer instead of
having to count characters by hand.
Verified against the reproducer pinned in the PR comment thread:
mkdir -p temp-invalid && python3 -c '...' && \
dart bin/cli.dart -s temp-invalid --valid-yaml-metadata; \
rm -rf temp-invalid
now emits 'Compatibility field is 501 characters; maximum is 500.
Cutoff at character 500: ...aaaaaaa|HERE|a (see ...)'.
Before: `dir[_pathKey] as String` (and the similar cast on ignore_file) threw at runtime when a user put a non-string value in their dart_skills_lint.yaml. The throw was caught by the top-level try/catch, but the side effect was that every subsequent directory entry in the same list was silently dropped from the configuration. After: each field is type-checked with `is String`; a bad type emits a Configuration.parsingErrors entry that names the offending field, the bad value, and its runtime type, and the entry is skipped. Later entries in the same `directories:` list still parse normally. New test pins the behavior with a config whose first entry uses `path: 123` and whose second entry is well-formed — the bad entry surfaces a parsing error without aborting the well-formed one.
Drops the prettier / eslint / ruff reference from the inline comment on --fix — it was decorative, not load-bearing, and the wording read as if fixes ran without an opt-in even though the user still has to pass the flag. Rewords the --fix flag's help text to a self-contained 'Write fixes for failing lints to disk. Combine with --dry-run to preview.' and tightens the --fix-apply deprecation notice to '--fix-apply is deprecated; use --fix instead. Pass --fix --dry-run to preview changes without writing.' — no more reference to --no-apply-fixes; that path was a dead end. Drops the `help:` argument from the _fixApplyFlag addFlag call. The flag is `hide: true`, so --help skips it anyway; carrying a help string there was misleading. A comment above the call says so out loud, and the runtime deprecation notice still covers adopters who hit the alias by name.
example_fixtures_test, rules_md_consistency_test, and a stale CLI test name still referenced T#/DT# tracking IDs that mean nothing to someone reading the test on its own. Replaced with feature-named prose that describes what the test actually exercises.
The shell-translation phase (rewriting 'dart pub global run dart_skills_lint <args>' to 'dart bin/cli.dart <args>' and replaying each step against the fixtures) was fragile and didn't catch anything that the structural YAML assertion didn't already cover. Dropped it. The remaining three tests read as assertions instead of as parsing code: (1) the README still has recipe blocks with non-empty bodies, (2) the workflow YAML still parses and still wires up the expected setup-dart + install + invocation steps, (3) the pre-commit hook body runs end-to-end against both example fixtures and exits with the right code. Parser/access helpers moved into a private _RecipeReader class so the test file's main() is a flat list of test() calls. Also drops a redundant async on a sibling-suggestion test.
The publish_dry_run job is gone. We'll add publish wiring when the publish itself is approved and not before; carrying a manual-only job just to watch it would be sitting code. The pana_score job is now one line: pana itself takes --exit-code-threshold and fails when (max - granted) exceeds it. The old jq/bash block that parsed grantedPoints out of the JSON report and conditionally exit-1'd is gone, and so is the artifact upload — pana is deterministic, so a maintainer investigating a score regression can reproduce it locally with one command instead of fishing the report out of a build artifact. Also reclaims the 10 points pana docked for an HTML-angle-bracket warning in cutoff_excerpt.dart's dartdoc placeholders by switching to backticks. Local pana is back to 160/160.
Version moves from 0.3.0 to 0.3.1 because 0.3.0 is already taken upstream by a different set of changes (ConfigParser API + tilde expansion + CLI-vs-test docs). Keeping 0.3.0 as-is and adding all of this PR's work as 0.3.1. The 0.3.1 entry only mentions things a consumer would care about: the new --fix semantics, the onboarding guide on first run, the improved diagnostics for the four most-hit rules, the new example fixtures, and the new README integration recipes. Cut: the SemVer-policy intro, the package-metadata bullets, the API restatement (already in 0.3.0), the CI section, and the forward-looking '1.0.0 — planned' block.
- Drops the 'As Dart Test Code' walkthrough and the 'Custom Rules' authoring section from the user-facing README. Custom-rule authoring stays available via the dart-skills-lint-validation skill, which a follow-up commit will sharpen. - Drops the 'CLI vs Dart Test' decision matrix at the top of Usage — the programmatic API isn't end-user-facing; contributor doc owns it now. - Drops the standalone Support section. The previous content documented Discussions setup that hasn't actually been enabled on the repo and a SECURITY.md path that doesn't exist; carrying it was promising channels we can't deliver. - Slims 'Specification Validation' from a four-section per-rule restate to a single paragraph that points at RULES.md, which is now the canonical rule reference. - Drops the trailing 'DT2' tracking reference in the pre-commit recipe; rewords the --fix flag entry to stop name-checking other ecosystem tools. - Trims the Table of Contents to match.
Adds a third recipe under ## Recipes — a copy-pasteable prompt that elevates the existing dart-skills-lint-setup and dart-skills-lint-validation skills. The prompt points the agent at both skills by repo-local path, so README readers with an agent delegate the wiring work; README readers without an agent still have the GitHub Actions and pre-commit recipes above for manual setup. recipe_drift_test gains a structural assertion that catches both skill paths going stale in the prompt — the prose lives in a blockquote rather than a fenced code block, so the test checks the raw README slice rather than running it.
- check-relative-paths: stops documenting the exact Levenshtein
threshold (max(1, basename.length / 3)) — that's an implementation
detail subject to tuning, not part of the rule's contract.
Replaced with prose about string similarity and the new
path-preserving suggestion behavior.
- check-absolute-paths: drops the dangling agentskills.io/...#content
link from the documented diagnostic to match the code.
- valid-yaml-metadata: updates the compatibility-length bullet to
reflect that it now uses the same buildLengthDiagnostic shape as
description-too-long ('N characters; maximum is 500. Cutoff at
character 500: ...|HERE|...').
RULES.md consistency test still passes (default-severity and
fixable claims unchanged).
dart-skills-lint-setup is now first-time-wiring only — adding the dep, creating dart_skills_lint.yaml, generating a baseline for legacy repos, and pointing at the README Recipes section for CI wiring. Dropped the inline GitHub Actions YAML and the validateSkills-in-tests snippet that duplicated the README. (~75 lines down from 116.) dart-skills-lint-validation is now day-to-day use only — running the linter (with a one-line CLI install branch instead of Scenario A/B), the run-review-fix-verify workflow, and the custom-rule authoring recipe. Dropped the Common Flags table (deferred to --help) and the Specification Reference (deferred to RULES.md). (~108 lines down from 121.) Each skill cross-links to the other for the next-step intent, and both now link to RULES.md as the canonical rule reference.
When the README's 'As Dart Test Code' section moved out, the ConfigParser.loadConfig + validateSkills + Validator API was left without a documented home. This is a contributor-facing topic (end users don't extend the linter), so it belongs here. The README points at this section by anchor.
Now that pana is at 160/160 we don't want any silent point drops. threshold 0 means (max - granted) > 0 fails the job, so any future regression in package metadata, docs, or static analysis surfaces immediately. Local pana still exits 0 at the new threshold.
The test was asserting the diagnostic contained 'resolved to /' to prove the resolved path was reported. That works on POSIX but fails on Windows where the absolute form starts with 'C:\'. Switched to extracting the resolved substring after 'resolved to ' and asserting p.isAbsolute() on it — same intent, no platform coupling.
reidbaker
commented
May 22, 2026
Each of the three regexes in the file now has a comment above it spelling out what input it matches, what gets captured into which group, and why the multiLine/dotAll flags are set. Future readers no longer have to mentally execute the pattern to understand it.
This was referenced May 25, 2026
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
2 participants
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.
Summary
The v0.3 prep cut for
tool/dart_skills_lint/. No rule semantics change — every skill that passed under 0.2.0 still passes under 0.3.0, with the same default severities. Everything below is paperwork, diagnostic polish, distribution prep, and developer experience.What changed
Better lint diagnostics
The four rules that adopters hit most often now produce error messages that actually point at the fix instead of restating the rule.
description-too-longnow reports the actual character count and shows a...before|HERE|after...excerpt around the 1024-char cutoff, so authors can see exactly where the text overflowed instead of counting characters by hand.invalid-skill-namedisambiguates every diagnostic withFrontmatter `name` "X"so it's never ambiguous whether the violation is inSKILL.mdor in the directory name. Each error suggests a normalized form (e.g.Suggested: "my-skill"), and the directory-mismatch error offers both fix directions (edit the field OR rename the dir) instead of silently preferring one.check-relative-pathsnow reports the resolved absolute path alongside the link as written, scans the parent directory for the nearest existing filename by Levenshtein distance, and surfaces aDid you mean "DETAILS.md"?suggestion when something close exists.check-absolute-pathsgained a one-line rationale (portability) and a spec link so authors don't have to guess why a hard-coded path is rejected.New CLI behavior
--fixnow applies fixes by default (matchesprettier --write/eslint --fix/ruff --fix). The new--fix --dry-runpreviews without writing. The old--fix-applystill works as a hidden alias and prints a deprecation notice to stderr — no existing pipelines break.Missing skills directoryerror. Still exits 64 so CI that depends on the failure mode doesn't silently start passing.New docs
tool/dart_skills_lint/example/— avalid/fixture that passes everything and aninvalid/fixture that deliberately trips three distinct rules, plus anexample/README.mdwalkthrough showing what the CLI actually prints. Pinned by a drift-guard test so the fixtures and their expected diagnostics can never desync.tool/dart_skills_lint/RULES.md— the rule contract: every built-in rule gets a section with default severity, fixable yes/no, exact diagnostic shape, auto-fix behavior, and the CLI flag to disable it. Pinned toRuleRegistryby a four-invariant consistency test (missing entry, orphan entry, severity mismatch, fixable mismatch) so a rule can't be added/removed/renamed/re-severitied without the docs catching up in the same commit.CONTRIBUTING.mdSemVer rule-stability policy — describes exactly what kind of rule change requires which version bump (patch = bug fixes & rewordings, minor = new rules defaulting to disabled, major = anything that can fail a previously-passing skill). Adopters can set^1.0.0and trust thatdart pub upgradenever turns green CI red.CHANGELOG.md— full 0.3.0 entry organized by surface (Package metadata, CLI, Rules, Documentation, CI), introducing a### Rulessubsection convention for future releases. Includes a1.0.0 — plannedsection anchoring the burn-in promise.Package & CI
pubspec.yamlbumped to0.3.0, description rewritten to the 50–180 char pana band, pub.devtopics:added,issue_tracker:added. Pana score jumped from 140/160 to 160/160 as a result.pana_scoreCI job runs pana on every PR, parsesgrantedPointsfrom the JSON report, and fails if the score drops below 150. Catches metadata / docs / static-analysis regressions before they hit pub.dev.publish_dry_runCI job runsdart pub publish --dry-runso maintainers can rehearse the v1.0.0 publish flow end-to-end. Gated onworkflow_dispatchonly, so it doesn't run on routine pushes.Health
dart analyze --fatal-infos— clean.dart format— clean.dart test— 126/126 pass (up from 110 before this PR; the new tests cover the new diagnostics, fixtures, RULES.md drift, and recipe drift).pana --no-warning .— 160/160.Test plan
analyze_and_testmatrix (ubuntu/macos/windows) green.pana_scorejob reports score ≥ 150.publish_dry_runjob is hidden (only runs onworkflow_dispatch, not on this PR).dart run dart_skills_lint --skill ./tool/dart_skills_lint/example/invalidagainst the new fixture shows the new diagnostic shapes (char count +|HERE|, frontmatter disambiguation, did-you-mean).dart run dart_skills_lint(no args, no.claude/skills) prints the new onboarding guide.RULES.mdcovers every rule that appears inRuleRegistry.allChecks.Out of scope
v0.3after merge — done manually.