Add skill-lint hook for dart_skills_lint by reidbaker · Pull Request #133 · flutter/skills

reidbaker · 2026-05-12T04:34:01Z

This pr is the result of me playing with gastown. It still needs review from me.

Summary

Adds an Antigravity hook that triggers on modifications to any SKILL.md file and runs dart_skills_lint against each affected skill directory in a single pass, surfacing lint errors via the standard JSON decision protocol.
New SkillLintHook subclasses BaseGitHook. It pre-filters scoped files by exact SKILL.md basename and maps each match to its parent skill directory before chunking, then invokes dart run --directory=<repo>/tool/dart_skills_lint bin/cli.dart -s <dir1> -s <dir2> ... once per chunk.
BaseGitHook gains two additive extension points so directory-oriented hooks don't have to duplicate run() orchestration:
- protected repoRoot field set in run() before any subclass hooks fire, for resolving repo-relative paths;
- protected transformScopedFiles virtual method (default identity) called between scope filtering and chunking.
Wired into .agents/hooks.json at all four existing layers (repo root, tool/, tool/dart_hooks/, tool/dart_skills_lint/) using the same relative-path scoping as dart-format and dart-analyze. Timeout 120s.

Test plan

5 unit tests for SkillLintHook (filter, dedup, success, failure, ignored non-SKILL.md files) and 2 integration tests against a real temp git repo. All pass.
All 3 existing BaseGitHook unit tests pass unchanged — the new extension points are additive.
dart analyze clean across tool/dart_hooks.

Note for reviewers

The new integration test calls tempDir.resolveSymbolicLinksSync() before assigning to repoRoot, because on macOS Directory.systemTemp returns /var/folders/... while git rev-parse --show-toplevel returns /private/var/folders/..., and the scope filter is a string-prefix comparison. The existing agent_dart_analyze_integration_test.dart and agent_dart_format_integration_test.dart don't apply this fix; a separate follow-up issue can address them.

Adds an Antigravity hook that triggers on modifications to any SKILL.md file and runs dart_skills_lint against each affected skill in a single pass, surfacing lint errors via the standard JSON decision protocol. Implementation notes: - New SkillLintHook subclasses BaseGitHook. It pre-filters scoped files by exact 'SKILL.md' basename and maps each match to its parent skill directory before chunking. executeCommand invokes `dart run --directory=<repo>/tool/dart_skills_lint bin/cli.dart -s ...` with one `-s` per skill directory. - BaseGitHook gains two additive extension points: * a protected `repoRoot` field, set in `run()` before any subclass hooks are called, so subclasses can resolve repo-relative paths; * a protected `transformScopedFiles` virtual method (default identity) called between scope filtering and chunking, so directory-oriented hooks can rewrite the file list without duplicating run() orchestration. - Wired into .agents/hooks.json at all four existing layers (repo root, tool/, tool/dart_hooks/, tool/dart_skills_lint/) using the same relative-path scoping as dart-format and dart-analyze. Timeout 120s. Tests: 5 unit tests for SkillLintHook (filter, dedup, success, failure, ignored .md files) and 2 integration tests against a real temp git repo (resolved-symlink repoRoot so the path matches `git rev-parse --show-toplevel` on macOS). All existing BaseGitHook tests continue to pass with the additive extension points.

gemini-code-assist

Code Review

This pull request introduces a new SkillLintHook designed to run dart_skills_lint on directories where SKILL.md files have been modified. To support this, the BaseGitHook class was updated with a repoRoot property and a transformScopedFiles method for customizing the file list before processing. Review feedback suggests removing the final modifier from repoRoot to avoid potential initialization errors, improving the chunking logic to account for argument overhead in subclasses, and considering case-insensitive matching for SKILL.md filenames.

BaseGitHook chunked by entry length plus a fixed +1 for the space separator. Subclasses that prepend per-entry flags (SkillLintHook adds '-s ' for every directory) undercounted by 3 characters per entry, which could push large chunks over the maxCharsPerChunk safety margin. Adds a protected 'perEntryArgOverhead' (default 0) that subclasses override to declare their flag overhead. SkillLintHook returns 3.

reidbaker · 2026-05-12T04:41:47Z

Addressing the gemini-code-assist review:

Remove final from repoRoot — keeping late final. Removing final allows accidental reassignment from subclasses; the one-time init pattern is what we want.
Chunking overhead — fixed in 90126c6. Added a perEntryArgOverhead extension point on BaseGitHook (default 0); SkillLintHook overrides to 3 for the -s prefix per entry, so chunks stay within the 4000-char safety margin.
Case-insensitive SKILL.md matching — keeping case-sensitive. dart_skills_lint itself requires exact SKILL.md, and the existing skills in the repo all use that casing. Loosening the hook would mean skill.md modifications get passed to the linter, which would then reject them anyway.

reidbaker · 2026-05-12T20:23:15Z

+  /// Defaults to 0 (no extra overhead). Override to return the number of
+  /// additional characters per entry, including any flag and its separator.
+  @protected
+  int get perEntryArgOverhead => 0;


I dont think this is needed especaialy if it is set to zero. Lets find a real world case before adding this complexity.

reidbaker · 2026-05-12T20:26:05Z

+      final List<String> transformedFiles = transformScopedFiles(scopedFiles);
+      if (transformedFiles.isEmpty) {
+        await logToFile('No files to process after transform.');
+        printStdout(jsonEncode({'decision': 'stop'}));


Lets extract printStdout(jsonEncode({'decision': 'stop'})); to a method that we can call on base_git_hook.dart when there is no more work to do.

reidbaker · 2026-05-12T20:26:47Z

-        // Add 1 for the space separator between arguments
-        final int fileLen = file.length + 1;
+      for (final file in transformedFiles) {
+        // Add 1 for the space separator between arguments, plus any


This will also go away after https://github.com/flutter/skills/pull/133/changes#r3229542020 is removed.

reidbaker · 2026-05-12T20:28:32Z

+  static const String _lintBinRelativePath = 'bin/cli.dart';
+
+  @override
+  List<String> get allowedExtensions => ['.md'];


where is this used?

reidbaker · 2026-05-12T20:31:08Z

+/// exactly `SKILL.md`, then runs the linter once with each skill's containing
+/// directory passed as a `-s` argument. A single pass; the agent is
+/// responsible for fixing reported errors.
+class SkillLintHook extends BaseGitHook {


How does this hook interact with a packages dart_skills_lint.yaml especially its ignore.json ?

reidbaker · 2026-05-12T20:32:05Z

+        }
+      }
+
+      await git(['init']);


I think there is already a helper to setup a bunch of these commands.

reidbaker · 2026-05-12T20:36:07Z

+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:convert';


This file looks like it has several places were we could reduce code duplication.

reidbaker · 2026-05-13T02:02:54Z

This comment was authored by an agent.

Addressed your review comments in 5bcbc42:

base_git_hook.dart:57 (perEntryArgOverhead) — reverted. Default-zero extension point with no real-world driver was unnecessary complexity; the chunking calc is back to file.length + 1.
base_git_hook.dart:124 (extract no-work emit) — added a private _emitNoMoreWork() on BaseGitHook that prints {"decision": "stop"} + exits 0. The three success/no-work call sites all route through it now.
base_git_hook.dart:142 — gone with the perEntryArgOverhead revert.
skill_lint_hook.dart:18 (yaml / ignore.json interaction) — added to the class doc comment. The hook invokes the CLI the way a developer would (dart run --directory=tool/dart_skills_lint bin/cli.dart -s <skill>), so dart_skills_lint.yaml and any configured ignore_file (e.g. flutter_skills_ignore.json) are discovered by the linter's normal rules. Ignored rules stay ignored through the hook.
skill_lint_hook.dart:38 (allowedExtensions — where is it used?) — used by BaseGitHook (via getModifiedFilesInternal) as a cheap pre-filter when walking git status --porcelain. The exact SKILL.md basename match happens later in transformScopedFiles. Added a doc comment near the override to make that explicit.
agent_skill_lint_integration_test.dart:37 (existing helper) — there wasn't one yet, so I added one: TempGitRepo in test_utils.dart (creates a temp git repo, runs the initial-commit boilerplate, exposes git(...), writeFile(...), stageAll(), dispose()). The integration test is now TempGitRepo.create() + a makeHook(...) builder, with all the git-setup boilerplate removed. The existing dart-analyze / dart-format integration tests still use their own inline setup — out of scope for this PR but a good candidate for fs-o01 (the macOS-symlink follow-up bead).
agent_skill_lint_test.dart:5 (unit-test duplication) — extracted makeMockedHook(...) plus a runWithModifiedFiles(...) helper at the group level. Each test is now just the modified-files input + the assertions, no repeated mock plumbing.

# Conflicts: # tool/dart_hooks/lib/src/base_git_hook.dart

reidbaker · 2026-05-15T18:26:21Z

/gemini review

gemini-code-assist

Code Review

This pull request adds a skill-lint hook to validate SKILL.md files using the dart_skills_lint tool. It refactors BaseGitHook to support file transformations and includes new tests for executable bits and integration. Reviewers recommended using the --directory flag in SkillLintHook for better dependency resolution and adjusting the chunking logic in BaseGitHook to account for per-file flags added by subclasses.

reidbaker added 2 commits May 12, 2026 00:26

Run dart format

486c00f

reidbaker marked this pull request as draft May 12, 2026 04:37

gemini-code-assist Bot reviewed May 12, 2026

View reviewed changes

Comment thread tool/dart_hooks/lib/src/base_git_hook.dart Outdated

Comment thread tool/dart_hooks/lib/src/base_git_hook.dart

Comment thread tool/dart_hooks/lib/src/skill_lint_hook.dart Outdated

reidbaker commented May 12, 2026

View reviewed changes

reidbaker added 2 commits May 12, 2026 17:34

Refactor git hook infrastructure and address PR 133 review feedback

101d38f

Update best practices and test fundamentals skills

aebbfbb

reidbaker added 2 commits May 13, 2026 11:02

Fix ai made up --directory flag

a8b3c48

Fix non executing skill lint, author skill for adding hooks to repo

9db3a5e

reidbaker force-pushed the r-skill-lint-hook branch from 5bcbc42 to 9db3a5e Compare May 14, 2026 13:39

Merge remote-tracking branch 'upstream/main' into r-skill-lint-hook

5e741f8

# Conflicts: # tool/dart_hooks/lib/src/base_git_hook.dart

gemini-code-assist Bot reviewed May 15, 2026

View reviewed changes

Comment thread tool/dart_hooks/lib/src/skill_lint_hook.dart

Comment thread tool/dart_hooks/lib/src/base_git_hook.dart

Conversation

reidbaker commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Note for reviewers

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reidbaker commented May 12, 2026

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker May 12, 2026

Choose a reason for hiding this comment

Uh oh!

reidbaker commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reidbaker commented May 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

reidbaker commented May 12, 2026 •

edited

Loading

reidbaker commented May 13, 2026 •

edited

Loading