From b5263ef6a6678c87777a685ba714734de5840484 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:26:45 -0400 Subject: [PATCH 1/7] Add skill-lint hook for dart_skills_lint 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=/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. --- .agents/hooks.json | 9 + tool/.agents/hooks.json | 9 + tool/dart_hooks/.agents/hooks.json | 9 + tool/dart_hooks/bin/agent_skill_lint.dart | 29 +++ tool/dart_hooks/lib/src/base_git_hook.dart | 29 ++- tool/dart_hooks/lib/src/skill_lint_hook.dart | 68 ++++++ .../agent_skill_lint_integration_test.dart | 152 ++++++++++++ .../test/agent_skill_lint_test.dart | 216 ++++++++++++++++++ tool/dart_skills_lint/.agents/hooks.json | 9 + 9 files changed, 527 insertions(+), 3 deletions(-) create mode 100644 tool/dart_hooks/bin/agent_skill_lint.dart create mode 100644 tool/dart_hooks/lib/src/skill_lint_hook.dart create mode 100644 tool/dart_hooks/test/agent_skill_lint_integration_test.dart create mode 100644 tool/dart_hooks/test/agent_skill_lint_test.dart diff --git a/.agents/hooks.json b/.agents/hooks.json index d105745..40dd0a1 100644 --- a/.agents/hooks.json +++ b/.agents/hooks.json @@ -16,5 +16,14 @@ "timeout": 90 } ] + }, + "skill-lint": { + "Stop": [ + { + "type": "command", + "command": "../tool/dart_hooks/bin/agent_skill_lint.dart --source hook --log", + "timeout": 120 + } + ] } } diff --git a/tool/.agents/hooks.json b/tool/.agents/hooks.json index f383ce3..3b5d503 100644 --- a/tool/.agents/hooks.json +++ b/tool/.agents/hooks.json @@ -16,5 +16,14 @@ "timeout": 90 } ] + }, + "skill-lint": { + "Stop": [ + { + "type": "command", + "command": "../dart_hooks/bin/agent_skill_lint.dart --source hook --log", + "timeout": 120 + } + ] } } diff --git a/tool/dart_hooks/.agents/hooks.json b/tool/dart_hooks/.agents/hooks.json index 0f43979..ede7835 100644 --- a/tool/dart_hooks/.agents/hooks.json +++ b/tool/dart_hooks/.agents/hooks.json @@ -16,5 +16,14 @@ "timeout": 90 } ] + }, + "skill-lint": { + "Stop": [ + { + "type": "command", + "command": "../bin/agent_skill_lint.dart --source hook --log", + "timeout": 120 + } + ] } } diff --git a/tool/dart_hooks/bin/agent_skill_lint.dart b/tool/dart_hooks/bin/agent_skill_lint.dart new file mode 100644 index 0000000..35de51b --- /dev/null +++ b/tool/dart_hooks/bin/agent_skill_lint.dart @@ -0,0 +1,29 @@ +#!/usr/bin/env dart +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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:io'; +import 'package:dart_hooks/src/hook_utils.dart'; +import 'package:dart_hooks/src/skill_lint_hook.dart'; + +/// Runs `dart_skills_lint` against any skill whose `SKILL.md` was modified. +/// Typically invoked automatically by Antigravity via `.agents/hooks.json`. +/// To run manually, execute from the project root: +/// `dart tool/dart_hooks/bin/agent_skill_lint.dart` +Future main(List args) async { + await runHookMain( + args: args, + logFileName: 'skill_lint.log', + executeHook: (source, logToFile) async { + final String packageRoot = Directory.current.parent.path; + final hook = SkillLintHook(logToFile: logToFile); + await hook.run( + args: args, + currentPath: Directory.current.path, + packageRoot: packageRoot, + triggerSource: source, + ); + }, + ); +} diff --git a/tool/dart_hooks/lib/src/base_git_hook.dart b/tool/dart_hooks/lib/src/base_git_hook.dart index 111fd35..1e09a1a 100644 --- a/tool/dart_hooks/lib/src/base_git_hook.dart +++ b/tool/dart_hooks/lib/src/base_git_hook.dart @@ -31,6 +31,21 @@ abstract class BaseGitHook { /// The name of the hook for logging purposes. String get hookName; + /// Absolute path to the repository root. Set during [run] before any + /// invocation of [executeCommand] or [transformScopedFiles]. + @protected + late final String repoRoot; + + /// Optional hook for subclasses to rewrite the scoped file list before + /// chunking. The default implementation is the identity function. + /// + /// Subclasses can use this to apply additional filtering (e.g., matching + /// only specific basenames) or to map file paths to other arguments + /// (e.g., mapping `path/to/SKILL.md` to `path/to`). Returning an empty + /// list short-circuits the hook to `{"decision": "stop"}`. + @protected + List transformScopedFiles(List scopedFiles) => scopedFiles; + /// Runs the specific command on the files (e.g., `dart analyze`). @protected Future executeCommand(List files); @@ -57,7 +72,7 @@ abstract class BaseGitHook { onExit(0); return; } - final String repoRoot = (repoRootResult.stdout as String).trim(); + repoRoot = (repoRootResult.stdout as String).trim(); // 2. Get modified files final List files; @@ -93,7 +108,15 @@ abstract class BaseGitHook { return; } - await logToFile('Running command on ${scopedFiles.length} files...'); + final List transformedFiles = transformScopedFiles(scopedFiles); + if (transformedFiles.isEmpty) { + await logToFile('No files to process after transform.'); + printStdout(jsonEncode({'decision': 'stop'})); + onExit(0); + return; + } + + await logToFile('Running command on ${transformedFiles.length} files...'); // 4. Execute the specific command in chunks to avoid ARG_MAX limits. // Determining the exact ARG_MAX is hard as it varies by OS and depends on environment size. @@ -105,7 +128,7 @@ abstract class BaseGitHook { var currentChunk = []; var currentChunkLength = 0; - for (final file in scopedFiles) { + for (final file in transformedFiles) { // Add 1 for the space separator between arguments final int fileLen = file.length + 1; diff --git a/tool/dart_hooks/lib/src/skill_lint_hook.dart b/tool/dart_hooks/lib/src/skill_lint_hook.dart new file mode 100644 index 0000000..08f97b2 --- /dev/null +++ b/tool/dart_hooks/lib/src/skill_lint_hook.dart @@ -0,0 +1,68 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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:io'; +import 'package:path/path.dart' as p; +import 'base_git_hook.dart'; +import 'process_runner.dart'; + +/// Implements a hook that runs `dart_skills_lint` against any skill whose +/// `SKILL.md` was modified. +/// +/// Unlike file-oriented hooks (analyze, format), this hook is skill-directory +/// oriented: it filters the modified-file list to entries whose basename is +/// 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 { + /// Creates a [SkillLintHook]. + SkillLintHook({ + super.processRunner = const RealProcessRunner(), + super.fileExists = _defaultFileExists, + super.printStdout = _defaultPrintStdout, + required super.logToFile, + super.onExit = exit, + }); + + static bool _defaultFileExists(String path) => File(path).existsSync(); + static void _defaultPrintStdout(String message) => stdout.writeln(message); + + /// Path to the `dart_skills_lint` package, relative to the repository root. + static const String _lintPackageRelativePath = 'tool/dart_skills_lint'; + + /// CLI entrypoint inside the `dart_skills_lint` package. + static const String _lintBinRelativePath = 'bin/cli.dart'; + + @override + List get allowedExtensions => ['.md']; + + @override + String get hookName => 'dart_skills_lint'; + + /// Filters the scoped file list to entries whose basename is exactly + /// `SKILL.md`, then maps each to its parent directory. Duplicates are + /// removed and the result is sorted for deterministic command-line output. + @override + List transformScopedFiles(List scopedFiles) { + final skillDirectories = {}; + for (final file in scopedFiles) { + if (p.basename(file) == 'SKILL.md') { + skillDirectories.add(p.normalize(p.dirname(file))); + } + } + return skillDirectories.toList()..sort(); + } + + @override + Future executeCommand(List skillDirectories) { + final String lintPackageDir = p.join(repoRoot, _lintPackageRelativePath); + final args = [ + 'run', + '--directory=$lintPackageDir', + _lintBinRelativePath, + for (final dir in skillDirectories) ...['-s', dir], + ]; + return processRunner.run('dart', args); + } +} diff --git a/tool/dart_hooks/test/agent_skill_lint_integration_test.dart b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart new file mode 100644 index 0000000..13df419 --- /dev/null +++ b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart @@ -0,0 +1,152 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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'; +import 'dart:io'; + +import 'package:dart_hooks/src/skill_lint_hook.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; + +import 'test_utils.dart'; + +void main() { + group('SkillLintHook Integration Tests', () { + late Directory tempDir; + late String repoRoot; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('skill_lint_test_'); + // Resolve symlinks so the path matches what `git rev-parse --show-toplevel` + // reports (macOS symlinks /var → /private/var under /tmp). + repoRoot = tempDir.resolveSymbolicLinksSync(); + + Future git(List args) async { + final ProcessResult r = await Process.run( + 'git', + args, + workingDirectory: repoRoot, + runInShell: true, + ); + if (r.exitCode != 0) { + throw Exception('git ${args.join(' ')} failed: ${r.stderr}'); + } + } + + await git(['init']); + await git(['config', 'user.email', 'test@example.com']); + await git(['config', 'user.name', 'Test User']); + + await File(path.join(repoRoot, 'README.md')).writeAsString('initial'); + await git(['add', '.']); + await git(['commit', '-m', 'initial']); + }); + + tearDown(() async { + await tempDir.delete(recursive: true); + }); + + test('runs lint on modified SKILL.md, ignoring other .md files', () async { + // Two skills with SKILL.md, plus a sibling README.md that should be + // ignored. + final foo = File(path.join(repoRoot, 'skills', 'foo', 'SKILL.md')); + await foo.create(recursive: true); + await foo.writeAsString('foo'); + + final bar = File(path.join(repoRoot, 'skills', 'bar', 'SKILL.md')); + await bar.create(recursive: true); + await bar.writeAsString('bar'); + + final readme = File(path.join(repoRoot, 'skills', 'foo', 'README.md')); + await readme.writeAsString('readme'); + + await Process.run('git', ['add', '.'], workingDirectory: repoRoot, runInShell: true); + + List? dartArgs; + String? stdoutMessage; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'dart' && args.first == 'run') { + dartArgs = args; + return ProcessResult(0, 0, 'All skills passed.', ''); + } + return Process.run( + cmd, + args, + runInShell: runInShell, + workingDirectory: workingDirectory ?? repoRoot, + ); + }), + fileExists: (p) => File(p).existsSync(), + printStdout: (msg) => stdoutMessage = msg, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: repoRoot, + packageRoot: repoRoot, + triggerSource: 'MANUAL', + ); + + expect(dartArgs, isNotNull, reason: 'dart_skills_lint must run'); + expect(dartArgs, contains('--directory=${path.join(repoRoot, 'tool/dart_skills_lint')}')); + // -s should appear for foo and bar only (not for README.md's dir). + final sTargets = []; + for (var i = 0; i < dartArgs!.length - 1; i++) { + if (dartArgs![i] == '-s') { + sTargets.add(dartArgs![i + 1]); + } + } + expect(sTargets, hasLength(2)); + expect(sTargets, contains(path.join(repoRoot, 'skills', 'foo'))); + expect(sTargets, contains(path.join(repoRoot, 'skills', 'bar'))); + expect(stdoutMessage, equals(jsonEncode({'decision': 'stop'}))); + expect(exitCode, equals(0)); + }); + + test('reports lint errors via continue decision', () async { + final foo = File(path.join(repoRoot, 'skills', 'foo', 'SKILL.md')); + await foo.create(recursive: true); + await foo.writeAsString('trailing whitespace '); + + await Process.run('git', ['add', '.'], workingDirectory: repoRoot, runInShell: true); + + String? stdoutMessage; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'dart' && args.first == 'run') { + return ProcessResult(0, 1, 'skills/foo: trailing whitespace on line 1', ''); + } + return Process.run( + cmd, + args, + runInShell: runInShell, + workingDirectory: workingDirectory ?? repoRoot, + ); + }), + fileExists: (p) => File(p).existsSync(), + printStdout: (msg) => stdoutMessage = msg, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: repoRoot, + packageRoot: repoRoot, + triggerSource: 'MANUAL', + ); + + expect(stdoutMessage, contains('"decision":"continue"')); + expect(stdoutMessage, contains('trailing whitespace on line 1')); + expect(exitCode, equals(0)); + }); + }); +} diff --git a/tool/dart_hooks/test/agent_skill_lint_test.dart b/tool/dart_hooks/test/agent_skill_lint_test.dart new file mode 100644 index 0000000..9906eaf --- /dev/null +++ b/tool/dart_hooks/test/agent_skill_lint_test.dart @@ -0,0 +1,216 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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'; +import 'dart:io'; + +import 'package:dart_hooks/src/skill_lint_hook.dart'; +import 'package:test/test.dart'; + +import 'test_utils.dart'; + +void main() { + group('SkillLintHook Unit Tests', () { + test('runs dart_skills_lint with -s for each modified SKILL.md', () async { + List? dartArgs; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + return ProcessResult( + 0, + 0, + 'M skills/foo/SKILL.md\x00M skills/bar/SKILL.md\x00', + '', + ); + } + if (cmd == 'dart' && args.first == 'run') { + dartArgs = args; + return ProcessResult(0, 0, '', ''); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: (msg) {}, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(exitCode, equals(0)); + expect(dartArgs, isNotNull); + expect(dartArgs!.first, equals('run')); + expect(dartArgs, contains('--directory=/repo/root/tool/dart_skills_lint')); + expect(dartArgs, contains('bin/cli.dart')); + // Each skill dir is preceded by -s. + expect(dartArgs, containsAllInOrder(['-s', '/repo/root/skills/bar'])); + expect(dartArgs, containsAllInOrder(['-s', '/repo/root/skills/foo'])); + }); + + test('ignores .md files that are not SKILL.md', () async { + List? dartArgs; + String? stdoutMessage; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + return ProcessResult( + 0, + 0, + 'M skills/foo/README.md\x00M skills/foo/skill.md\x00M docs/CONTRIBUTING.md\x00', + '', + ); + } + if (cmd == 'dart' && args.first == 'run') { + dartArgs = args; + return ProcessResult(0, 0, '', ''); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: (msg) => stdoutMessage = msg, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(dartArgs, isNull, reason: 'dart_skills_lint must not run when no SKILL.md changed'); + expect(stdoutMessage, equals(jsonEncode({'decision': 'stop'}))); + expect(exitCode, equals(0)); + }); + + test('deduplicates skill directory when nested files also match', () async { + List? dartArgs; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + // Two SKILL.md modifications in different skill dirs, both should + // appear once (no accidental duplication from path normalization). + return ProcessResult( + 0, + 0, + 'M skills/foo/SKILL.md\x00M skills/foo/SKILL.md\x00', + '', + ); + } + if (cmd == 'dart' && args.first == 'run') { + dartArgs = args; + return ProcessResult(0, 0, '', ''); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: (msg) {}, + logToFile: (msg) async {}, + onExit: (code) {}, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(dartArgs, isNotNull); + // Count -s occurrences: should be exactly 1. + final int sFlagCount = dartArgs!.where((a) => a == '-s').length; + expect(sFlagCount, equals(1)); + }); + + test('returns continue decision on lint failure', () async { + String? stdoutMessage; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00', ''); + } + if (cmd == 'dart' && args.first == 'run') { + return ProcessResult(0, 1, 'foo: trailing whitespace on line 3', ''); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: (msg) => stdoutMessage = msg, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(stdoutMessage, contains('"decision":"continue"')); + expect(stdoutMessage, contains('trailing whitespace on line 3')); + // Antigravity captures stdout JSON only when the hook exits 0. + expect(exitCode, equals(0)); + }); + + test('returns stop decision when SKILL.md modifications all pass lint', () async { + String? stdoutMessage; + int? exitCode; + + final hook = SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00', ''); + } + if (cmd == 'dart' && args.first == 'run') { + return ProcessResult(0, 0, 'All skills passed.', ''); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: (msg) => stdoutMessage = msg, + logToFile: (msg) async {}, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(stdoutMessage, equals(jsonEncode({'decision': 'stop'}))); + expect(exitCode, equals(0)); + }); + }); +} diff --git a/tool/dart_skills_lint/.agents/hooks.json b/tool/dart_skills_lint/.agents/hooks.json index 175b651..a4ad111 100644 --- a/tool/dart_skills_lint/.agents/hooks.json +++ b/tool/dart_skills_lint/.agents/hooks.json @@ -16,5 +16,14 @@ "timeout": 90 } ] + }, + "skill-lint": { + "Stop": [ + { + "type": "command", + "command": "../../dart_hooks/bin/agent_skill_lint.dart --source hook --log", + "timeout": 120 + } + ] } } From 486c00fac6bdc29bd29b7a5f548c10a5e0eec0e9 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:36:08 -0400 Subject: [PATCH 2/7] Run dart format --- tool/dart_hooks/test/agent_skill_lint_test.dart | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tool/dart_hooks/test/agent_skill_lint_test.dart b/tool/dart_hooks/test/agent_skill_lint_test.dart index 9906eaf..c1acf4b 100644 --- a/tool/dart_hooks/test/agent_skill_lint_test.dart +++ b/tool/dart_hooks/test/agent_skill_lint_test.dart @@ -22,12 +22,7 @@ void main() { return ProcessResult(0, 0, '/repo/root', ''); } if (cmd == 'git' && args.first == 'status') { - return ProcessResult( - 0, - 0, - 'M skills/foo/SKILL.md\x00M skills/bar/SKILL.md\x00', - '', - ); + return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00M skills/bar/SKILL.md\x00', ''); } if (cmd == 'dart' && args.first == 'run') { dartArgs = args; @@ -111,12 +106,7 @@ void main() { if (cmd == 'git' && args.first == 'status') { // Two SKILL.md modifications in different skill dirs, both should // appear once (no accidental duplication from path normalization). - return ProcessResult( - 0, - 0, - 'M skills/foo/SKILL.md\x00M skills/foo/SKILL.md\x00', - '', - ); + return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00M skills/foo/SKILL.md\x00', ''); } if (cmd == 'dart' && args.first == 'run') { dartArgs = args; From 90126c65af2a3ec98ab56956339da443d7e9ab3d Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 00:41:36 -0400 Subject: [PATCH 3/7] Address review: account for subclass arg overhead in chunking 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. --- tool/dart_hooks/lib/src/base_git_hook.dart | 15 +++++++++++++-- tool/dart_hooks/lib/src/skill_lint_hook.dart | 5 +++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tool/dart_hooks/lib/src/base_git_hook.dart b/tool/dart_hooks/lib/src/base_git_hook.dart index 1e09a1a..954deb5 100644 --- a/tool/dart_hooks/lib/src/base_git_hook.dart +++ b/tool/dart_hooks/lib/src/base_git_hook.dart @@ -46,6 +46,16 @@ abstract class BaseGitHook { @protected List transformScopedFiles(List scopedFiles) => scopedFiles; + /// Extra per-entry argument overhead that [executeCommand] introduces + /// around each chunk entry. Counted by the chunker on top of the entry's + /// own length so subclasses that prepend flags (e.g., `-s `) don't + /// undercount and produce oversized command lines. + /// + /// 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; + /// Runs the specific command on the files (e.g., `dart analyze`). @protected Future executeCommand(List files); @@ -129,8 +139,9 @@ abstract class BaseGitHook { var currentChunkLength = 0; for (final file in transformedFiles) { - // Add 1 for the space separator between arguments - final int fileLen = file.length + 1; + // Add 1 for the space separator between arguments, plus any + // per-entry flag overhead the subclass introduces. + final int fileLen = file.length + 1 + perEntryArgOverhead; if (currentChunkLength + fileLen > maxCharsPerChunk && currentChunk.isNotEmpty) { await logToFile('Running command on chunk of ${currentChunk.length} files...'); diff --git a/tool/dart_hooks/lib/src/skill_lint_hook.dart b/tool/dart_hooks/lib/src/skill_lint_hook.dart index 08f97b2..3efd7b3 100644 --- a/tool/dart_hooks/lib/src/skill_lint_hook.dart +++ b/tool/dart_hooks/lib/src/skill_lint_hook.dart @@ -40,6 +40,11 @@ class SkillLintHook extends BaseGitHook { @override String get hookName => 'dart_skills_lint'; + /// Each entry is rendered as `-s `, which adds 3 characters + /// (`-`, `s`, space) on top of the directory path itself. + @override + int get perEntryArgOverhead => 3; + /// Filters the scoped file list to entries whose basename is exactly /// `SKILL.md`, then maps each to its parent directory. Duplicates are /// removed and the result is sorted for deterministic command-line output. From 101d38f5147027154d2136d4a611db646cc9390b Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 17:34:27 -0400 Subject: [PATCH 4/7] Refactor git hook infrastructure and address PR 133 review feedback --- tool/dart_hooks/lib/src/base_git_hook.dart | 29 ++-- tool/dart_hooks/lib/src/skill_lint_hook.dart | 11 +- .../agent_skill_lint_integration_test.dart | 23 +-- .../test/agent_skill_lint_test.dart | 161 +++++++++--------- tool/dart_hooks/test/test_utils.dart | 26 +++ 5 files changed, 118 insertions(+), 132 deletions(-) diff --git a/tool/dart_hooks/lib/src/base_git_hook.dart b/tool/dart_hooks/lib/src/base_git_hook.dart index 954deb5..d61b9ca 100644 --- a/tool/dart_hooks/lib/src/base_git_hook.dart +++ b/tool/dart_hooks/lib/src/base_git_hook.dart @@ -34,7 +34,7 @@ abstract class BaseGitHook { /// Absolute path to the repository root. Set during [run] before any /// invocation of [executeCommand] or [transformScopedFiles]. @protected - late final String repoRoot; + late String repoRoot; /// Optional hook for subclasses to rewrite the scoped file list before /// chunking. The default implementation is the identity function. @@ -46,15 +46,12 @@ abstract class BaseGitHook { @protected List transformScopedFiles(List scopedFiles) => scopedFiles; - /// Extra per-entry argument overhead that [executeCommand] introduces - /// around each chunk entry. Counted by the chunker on top of the entry's - /// own length so subclasses that prepend flags (e.g., `-s `) don't - /// undercount and produce oversized command lines. - /// - /// Defaults to 0 (no extra overhead). Override to return the number of - /// additional characters per entry, including any flag and its separator. + /// Prints a stop decision and invokes the exit callback with a success code. @protected - int get perEntryArgOverhead => 0; + void stopHook() { + printStdout(jsonEncode({'decision': 'stop'})); + onExit(0); + } /// Runs the specific command on the files (e.g., `dart analyze`). @protected @@ -113,16 +110,14 @@ abstract class BaseGitHook { if (scopedFiles.isEmpty) { await logToFile('No matching files found to process in scope: $scopeDir.'); - printStdout(jsonEncode({'decision': 'stop'})); - onExit(0); + stopHook(); return; } final List transformedFiles = transformScopedFiles(scopedFiles); if (transformedFiles.isEmpty) { await logToFile('No files to process after transform.'); - printStdout(jsonEncode({'decision': 'stop'})); - onExit(0); + stopHook(); return; } @@ -139,9 +134,8 @@ abstract class BaseGitHook { var currentChunkLength = 0; for (final file in transformedFiles) { - // Add 1 for the space separator between arguments, plus any - // per-entry flag overhead the subclass introduces. - final int fileLen = file.length + 1 + perEntryArgOverhead; + // Add 1 for the space separator between arguments. + final int fileLen = file.length + 1; if (currentChunkLength + fileLen > maxCharsPerChunk && currentChunk.isNotEmpty) { await logToFile('Running command on chunk of ${currentChunk.length} files...'); @@ -182,8 +176,7 @@ abstract class BaseGitHook { // 5. Handle result if (exitCode == 0) { await logToFile('Command passed'); - printStdout(jsonEncode({'decision': 'stop'})); - onExit(0); + stopHook(); return; } diff --git a/tool/dart_hooks/lib/src/skill_lint_hook.dart b/tool/dart_hooks/lib/src/skill_lint_hook.dart index 3efd7b3..df8a3fa 100644 --- a/tool/dart_hooks/lib/src/skill_lint_hook.dart +++ b/tool/dart_hooks/lib/src/skill_lint_hook.dart @@ -34,25 +34,22 @@ class SkillLintHook extends BaseGitHook { /// CLI entrypoint inside the `dart_skills_lint` package. static const String _lintBinRelativePath = 'bin/cli.dart'; + /// Filters the raw git status modified files by extension (e.g., ['.md']) before + /// scoping and chunking. @override List get allowedExtensions => ['.md']; @override String get hookName => 'dart_skills_lint'; - /// Each entry is rendered as `-s `, which adds 3 characters - /// (`-`, `s`, space) on top of the directory path itself. - @override - int get perEntryArgOverhead => 3; - - /// Filters the scoped file list to entries whose basename is exactly + /// Filters the scoped file list to entries whose basename is case-insensitively /// `SKILL.md`, then maps each to its parent directory. Duplicates are /// removed and the result is sorted for deterministic command-line output. @override List transformScopedFiles(List scopedFiles) { final skillDirectories = {}; for (final file in scopedFiles) { - if (p.basename(file) == 'SKILL.md') { + if (p.basename(file).toLowerCase() == 'skill.md') { skillDirectories.add(p.normalize(p.dirname(file))); } } diff --git a/tool/dart_hooks/test/agent_skill_lint_integration_test.dart b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart index 13df419..7e7c023 100644 --- a/tool/dart_hooks/test/agent_skill_lint_integration_test.dart +++ b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart @@ -18,29 +18,8 @@ void main() { setUp(() async { tempDir = await Directory.systemTemp.createTemp('skill_lint_test_'); - // Resolve symlinks so the path matches what `git rev-parse --show-toplevel` - // reports (macOS symlinks /var → /private/var under /tmp). repoRoot = tempDir.resolveSymbolicLinksSync(); - - Future git(List args) async { - final ProcessResult r = await Process.run( - 'git', - args, - workingDirectory: repoRoot, - runInShell: true, - ); - if (r.exitCode != 0) { - throw Exception('git ${args.join(' ')} failed: ${r.stderr}'); - } - } - - await git(['init']); - await git(['config', 'user.email', 'test@example.com']); - await git(['config', 'user.name', 'Test User']); - - await File(path.join(repoRoot, 'README.md')).writeAsString('initial'); - await git(['add', '.']); - await git(['commit', '-m', 'initial']); + await setUpGitRepo(tempDir); }); tearDown(() async { diff --git a/tool/dart_hooks/test/agent_skill_lint_test.dart b/tool/dart_hooks/test/agent_skill_lint_test.dart index c1acf4b..ed4210a 100644 --- a/tool/dart_hooks/test/agent_skill_lint_test.dart +++ b/tool/dart_hooks/test/agent_skill_lint_test.dart @@ -16,23 +16,12 @@ void main() { List? dartArgs; int? exitCode; - final hook = SkillLintHook( - processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { - if (cmd == 'git' && args.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/repo/root', ''); - } - if (cmd == 'git' && args.first == 'status') { - return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00M skills/bar/SKILL.md\x00', ''); - } - if (cmd == 'dart' && args.first == 'run') { - dartArgs = args; - return ProcessResult(0, 0, '', ''); - } + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/SKILL.md\x00M skills/bar/SKILL.md\x00', + onDartRun: (cmd, args) async { + dartArgs = args; return ProcessResult(0, 0, '', ''); - }), - fileExists: (path) => true, - printStdout: (msg) {}, - logToFile: (msg) async {}, + }, onExit: (code) => exitCode = code, ); @@ -58,28 +47,13 @@ void main() { String? stdoutMessage; int? exitCode; - final hook = SkillLintHook( - processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { - if (cmd == 'git' && args.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/repo/root', ''); - } - if (cmd == 'git' && args.first == 'status') { - return ProcessResult( - 0, - 0, - 'M skills/foo/README.md\x00M skills/foo/skill.md\x00M docs/CONTRIBUTING.md\x00', - '', - ); - } - if (cmd == 'dart' && args.first == 'run') { - dartArgs = args; - return ProcessResult(0, 0, '', ''); - } + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/README.md\x00M docs/CONTRIBUTING.md\x00', + onDartRun: (cmd, args) async { + dartArgs = args; return ProcessResult(0, 0, '', ''); - }), - fileExists: (path) => true, + }, printStdout: (msg) => stdoutMessage = msg, - logToFile: (msg) async {}, onExit: (code) => exitCode = code, ); @@ -95,29 +69,40 @@ void main() { expect(exitCode, equals(0)); }); + test('matches skill.md case-insensitively', () async { + List? dartArgs; + int? exitCode; + + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/skill.md\x00', + onDartRun: (cmd, args) async { + dartArgs = args; + return ProcessResult(0, 0, '', ''); + }, + onExit: (code) => exitCode = code, + ); + + await hook.run( + args: [], + currentPath: '/repo/root', + packageRoot: '/repo/root', + triggerSource: 'MANUAL', + ); + + expect(exitCode, equals(0)); + expect(dartArgs, isNotNull); + expect(dartArgs, containsAllInOrder(['-s', '/repo/root/skills/foo'])); + }); + test('deduplicates skill directory when nested files also match', () async { List? dartArgs; - final hook = SkillLintHook( - processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { - if (cmd == 'git' && args.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/repo/root', ''); - } - if (cmd == 'git' && args.first == 'status') { - // Two SKILL.md modifications in different skill dirs, both should - // appear once (no accidental duplication from path normalization). - return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00M skills/foo/SKILL.md\x00', ''); - } - if (cmd == 'dart' && args.first == 'run') { - dartArgs = args; - return ProcessResult(0, 0, '', ''); - } + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/SKILL.md\x00M skills/foo/SKILL.md\x00', + onDartRun: (cmd, args) async { + dartArgs = args; return ProcessResult(0, 0, '', ''); - }), - fileExists: (path) => true, - printStdout: (msg) {}, - logToFile: (msg) async {}, - onExit: (code) {}, + }, ); await hook.run( @@ -137,22 +122,12 @@ void main() { String? stdoutMessage; int? exitCode; - final hook = SkillLintHook( - processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { - if (cmd == 'git' && args.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/repo/root', ''); - } - if (cmd == 'git' && args.first == 'status') { - return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00', ''); - } - if (cmd == 'dart' && args.first == 'run') { - return ProcessResult(0, 1, 'foo: trailing whitespace on line 3', ''); - } - return ProcessResult(0, 0, '', ''); - }), - fileExists: (path) => true, + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/SKILL.md\x00', + onDartRun: (cmd, args) async { + return ProcessResult(0, 1, 'foo: trailing whitespace on line 3', ''); + }, printStdout: (msg) => stdoutMessage = msg, - logToFile: (msg) async {}, onExit: (code) => exitCode = code, ); @@ -173,22 +148,12 @@ void main() { String? stdoutMessage; int? exitCode; - final hook = SkillLintHook( - processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { - if (cmd == 'git' && args.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/repo/root', ''); - } - if (cmd == 'git' && args.first == 'status') { - return ProcessResult(0, 0, 'M skills/foo/SKILL.md\x00', ''); - } - if (cmd == 'dart' && args.first == 'run') { - return ProcessResult(0, 0, 'All skills passed.', ''); - } - return ProcessResult(0, 0, '', ''); - }), - fileExists: (path) => true, + final SkillLintHook hook = createHook( + gitStatusStdout: 'M skills/foo/SKILL.md\x00', + onDartRun: (cmd, args) async { + return ProcessResult(0, 0, 'All skills passed.', ''); + }, printStdout: (msg) => stdoutMessage = msg, - logToFile: (msg) async {}, onExit: (code) => exitCode = code, ); @@ -204,3 +169,29 @@ void main() { }); }); } + +SkillLintHook createHook({ + required String gitStatusStdout, + required Future Function(String cmd, List args) onDartRun, + void Function(String)? printStdout, + void Function(int)? onExit, +}) { + return SkillLintHook( + processRunner: MockProcessRunner((cmd, args, {runInShell = false, workingDirectory}) async { + if (cmd == 'git' && args.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/repo/root', ''); + } + if (cmd == 'git' && args.first == 'status') { + return ProcessResult(0, 0, gitStatusStdout, ''); + } + if (cmd == 'dart' && args.first == 'run') { + return onDartRun(cmd, args); + } + return ProcessResult(0, 0, '', ''); + }), + fileExists: (path) => true, + printStdout: printStdout ?? (msg) {}, + logToFile: (msg) async {}, + onExit: onExit ?? (code) {}, + ); +} diff --git a/tool/dart_hooks/test/test_utils.dart b/tool/dart_hooks/test/test_utils.dart index 4bf7840..c4bf14b 100644 --- a/tool/dart_hooks/test/test_utils.dart +++ b/tool/dart_hooks/test/test_utils.dart @@ -4,6 +4,32 @@ import 'dart:io'; import 'package:dart_hooks/src/process_runner.dart'; +import 'package:path/path.dart' as path; + +/// Initializes a temporary git repository with config email/name and a dummy initial commit. +Future setUpGitRepo(Directory tempDir) async { + final String repoRoot = tempDir.resolveSymbolicLinksSync(); + + Future git(List args) async { + final ProcessResult r = await Process.run( + 'git', + args, + workingDirectory: repoRoot, + runInShell: true, + ); + if (r.exitCode != 0) { + throw Exception('git ${args.join(' ')} failed: ${r.stderr}'); + } + } + + await git(['init']); + await git(['config', 'user.email', 'test@example.com']); + await git(['config', 'user.name', 'Test User']); + + await File(path.join(repoRoot, 'README.md')).writeAsString('initial'); + await git(['add', '.']); + await git(['commit', '-m', 'initial']); +} /// A mock implementation of [ProcessRunner] that delegates to a function. class MockProcessRunner implements ProcessRunner { From aebbfbb5ba5b4e72cb828abb82635d3ac5ed90f2 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 12 May 2026 17:36:24 -0400 Subject: [PATCH 5/7] Update best practices and test fundamentals skills --- .../skills/dart-best-practices/SKILL.md | 10 ++++++ .../skills/dart-test-fundamentals/SKILL.md | 33 +++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tool/dart_skills_lint/.agents/skills/dart-best-practices/SKILL.md b/tool/dart_skills_lint/.agents/skills/dart-best-practices/SKILL.md index 690cce7..7703db4 100644 --- a/tool/dart_skills_lint/.agents/skills/dart-best-practices/SKILL.md +++ b/tool/dart_skills_lint/.agents/skills/dart-best-practices/SKILL.md @@ -45,6 +45,16 @@ without horizontal scrolling. Target 80 characters for wrapping text. Exceptions are allowed for long URLs or identifiers that cannot be broken. +### Filesystem Case-Sensitivity (Scanner / Git Hooks) +On case-insensitive filesystems (like macOS and Windows), a file named `skill.md` or `Skill.md` resolves successfully in local test executions, but will fail on case-sensitive CI systems (like Ubuntu Linux) that strictly enforce the spec's uppercase `SKILL.md` casing. + +**Prefer:** +When scanning for specific basenames inside Git pre-commit hooks or validation engines, perform a case-insensitive check: +```dart +if (p.basename(file).toLowerCase() == 'skill.md') { ... } +``` +This forces local checks to trigger and correctly surface capitalization errors locally before code is pushed to CI. + ## Discovery ### Multi-line Strings diff --git a/tool/dart_skills_lint/.agents/skills/dart-test-fundamentals/SKILL.md b/tool/dart_skills_lint/.agents/skills/dart-test-fundamentals/SKILL.md index 39390dd..df181a7 100644 --- a/tool/dart_skills_lint/.agents/skills/dart-test-fundamentals/SKILL.md +++ b/tool/dart_skills_lint/.agents/skills/dart-test-fundamentals/SKILL.md @@ -108,7 +108,36 @@ test('can create and delete a file', () { }); ``` -### 4. Configuration (`dart_test.yaml`) +### 4. Minimizing Test Boilerplate + +When writing unit and integration tests, setup boilerplate (like mock environment provisioning or complex class configurations) can quickly duplicate across test blocks. Abstract this redundant code into shared test factories or setup helpers. + +#### Shared Environment Setup +If multiple integration tests require complex mock environment prep (such as Git repository configuration, directory structures, or mock users), move it into a shared `setUpGitRepo()` helper inside a standard `test_utils.dart` file: +```dart +Future setUpGitRepo(Directory tempDir) async { + final String repoRoot = tempDir.resolveSymbolicLinksSync(); + // Run git init, git config, create initial commit, etc. +} +``` + +#### Test-Local Boilerplate Factories +If tests require constructing mocked class instances with extensive runner parameters, define a helper factory (e.g., `createMockedInstance()`) that encapsulates default process executions and accepts specific assert/execute closures as parameters: +```dart +MyClass createInstance({ + required String statusPayload, + required Future Function(List args) onExecute, + void Function(int)? onExit, +}) { + return MyClass( + runner: MockRunner((args) => onExecute(args)), + onExit: onExit ?? (_) {}, + ); +} +``` +This localizes mocking logic, isolates the assert definitions, and vastly increases suite legibility. + +### 5. Configuration (`dart_test.yaml`) The `dart_test.yaml` file configures the test runner. Common configurations include: @@ -148,7 +177,7 @@ timeouts: 2x # Double the default timeout ``` -### 5. File Naming +### 6. File Naming - Test files **must** end in `_test.dart` to be picked up by the test runner. - Place tests in the `test/` directory. From a8b3c48c8bdc2e1243398c39a58fcf96fed7ffad Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Wed, 13 May 2026 11:02:38 -0400 Subject: [PATCH 6/7] Fix ai made up --directory flag --- tool/dart_hooks/lib/src/skill_lint_hook.dart | 4 ++-- tool/dart_hooks/test/agent_skill_lint_integration_test.dart | 2 +- tool/dart_hooks/test/agent_skill_lint_test.dart | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tool/dart_hooks/lib/src/skill_lint_hook.dart b/tool/dart_hooks/lib/src/skill_lint_hook.dart index df8a3fa..ccdab85 100644 --- a/tool/dart_hooks/lib/src/skill_lint_hook.dart +++ b/tool/dart_hooks/lib/src/skill_lint_hook.dart @@ -59,10 +59,10 @@ class SkillLintHook extends BaseGitHook { @override Future executeCommand(List skillDirectories) { final String lintPackageDir = p.join(repoRoot, _lintPackageRelativePath); + final String lintBinPath = p.join(lintPackageDir, _lintBinRelativePath); final args = [ 'run', - '--directory=$lintPackageDir', - _lintBinRelativePath, + lintBinPath, for (final dir in skillDirectories) ...['-s', dir], ]; return processRunner.run('dart', args); diff --git a/tool/dart_hooks/test/agent_skill_lint_integration_test.dart b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart index 7e7c023..729eacf 100644 --- a/tool/dart_hooks/test/agent_skill_lint_integration_test.dart +++ b/tool/dart_hooks/test/agent_skill_lint_integration_test.dart @@ -73,7 +73,7 @@ void main() { ); expect(dartArgs, isNotNull, reason: 'dart_skills_lint must run'); - expect(dartArgs, contains('--directory=${path.join(repoRoot, 'tool/dart_skills_lint')}')); + expect(dartArgs, contains(path.join(repoRoot, 'tool/dart_skills_lint/bin/cli.dart'))); // -s should appear for foo and bar only (not for README.md's dir). final sTargets = []; for (var i = 0; i < dartArgs!.length - 1; i++) { diff --git a/tool/dart_hooks/test/agent_skill_lint_test.dart b/tool/dart_hooks/test/agent_skill_lint_test.dart index ed4210a..feee405 100644 --- a/tool/dart_hooks/test/agent_skill_lint_test.dart +++ b/tool/dart_hooks/test/agent_skill_lint_test.dart @@ -35,8 +35,7 @@ void main() { expect(exitCode, equals(0)); expect(dartArgs, isNotNull); expect(dartArgs!.first, equals('run')); - expect(dartArgs, contains('--directory=/repo/root/tool/dart_skills_lint')); - expect(dartArgs, contains('bin/cli.dart')); + expect(dartArgs, contains('/repo/root/tool/dart_skills_lint/bin/cli.dart')); // Each skill dir is preceded by -s. expect(dartArgs, containsAllInOrder(['-s', '/repo/root/skills/bar'])); expect(dartArgs, containsAllInOrder(['-s', '/repo/root/skills/foo'])); From 9db3a5e6f795b641becff9b9a2039b1e4ed3b932 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Wed, 13 May 2026 12:29:14 -0400 Subject: [PATCH 7/7] Fix non executing skill lint, author skill for adding hooks to repo --- .../.agents/skills/author-agent-hook/SKILL.md | 75 +++++++++++++++++++ tool/dart_hooks/bin/agent_skill_lint.dart | 0 tool/dart_hooks/test/bin_executable_test.dart | 38 ++++++++++ 3 files changed, 113 insertions(+) create mode 100644 tool/dart_hooks/.agents/skills/author-agent-hook/SKILL.md mode change 100644 => 100755 tool/dart_hooks/bin/agent_skill_lint.dart create mode 100644 tool/dart_hooks/test/bin_executable_test.dart diff --git a/tool/dart_hooks/.agents/skills/author-agent-hook/SKILL.md b/tool/dart_hooks/.agents/skills/author-agent-hook/SKILL.md new file mode 100644 index 0000000..2d5b9b5 --- /dev/null +++ b/tool/dart_hooks/.agents/skills/author-agent-hook/SKILL.md @@ -0,0 +1,75 @@ +--- +name: author-agent-hook +description: Helps scaffold deterministic script execution triggered by agent lifecycle events (jetski/antigravity hooks) against a minimal set of changes. Make sure to invoke this skill eagerly whenever a user mentions they want to author a hook, automate tasks/scripts on every change, integrate custom scripts/linters into the agent loop, or set up event handlers inside hooks.json, even if they don't explicitly ask for 'hook scaffolding.' +--- + +# Authoring Agent Lifecycle Hooks (`author-agent-hook`) + +This skill establishes standard, deterministic scaffolding to execute a user-provided script or command during specific agent lifecycle events within the `dart_hooks` repository. + +## 1. Initial Context Gathering +Before authoring code, confirm with the user: +- **Target Script/Command**: The exact path or command string the user wants to execute. +- **Lifecycle Event Type**: The target event type (`PreToolUse`, `PostToolUse`, `PreInvocation`, `PostInvocation`, or `Stop`). If the user does not specify or does not know the event type, **assume `"Stop"`** by default. + +## 2. Scaffolding Implementation Details +Implement the hook functionality by generating the following standard file structure: + +### A. Executable Runner Script (`bin/agent_.dart`) +Create a thin entry point script inside the `bin/` directory delegating execution to the shared `runHookMain` utility. +- **CRITICAL**: Ensure the script contains a proper shebang (`#!/usr/bin/env dart`). +- **CRITICAL**: Ensure the script file has POSIX executable permissions enabled (`chmod +x`). Without execution bits, the shell will reject execution with `Permission denied` (exit code 126) when triggered via `hooks.json`. +- **Implementation Pattern**: +```dart +#!/usr/bin/env dart +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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:io'; +import 'package:dart_hooks/src/_hook.dart'; +import 'package:dart_hooks/src/hook_utils.dart'; + +Future main(List args) async { + await runHookMain( + args: args, + logFileName: '.log', + executeHook: (String source, Future Function(String) logToFile) async { + final String packageRoot = Directory.current.parent.path; + final hook = (logToFile: logToFile); + await hook.run( + args: args, + currentPath: Directory.current.path, + packageRoot: packageRoot, + triggerSource: source, + ); + }, + ); +} +``` + +### B. Core Hook Subclass (`lib/src/_hook.dart`) +Implement the custom hook logic by extending `BaseGitHook`. +- Provide standard overrides for `allowedExtensions`, `hookName`, and `executeCommand`. +- If the target script needs to process specific filtered paths or directories, override `transformScopedFiles` to map scoped files to the target command arguments. + +### C. Configuration Registration (`.agents/hooks.json`) +Register the hook under the user-specified (or defaulted `"Stop"`) event type key inside `.agents/hooks.json`. +- **Command String Details**: Format the command string exactly as required for direct execution via `sh -c`. +```json +"": { + "": [ + { + "type": "command", + "command": "../bin/agent_.dart --source hook --log", + "timeout": 120 + } + ] +} +``` +*(Note: For `Stop` events, handlers use a flat array structure directly under the event key without `matcher` or nested `hooks` wrappers).* + +## 3. Static Analysis & Testing Hygiene +Ensure all generated code strictly adheres to repository static analysis standards: +- **Typing Rules**: Run `dart analyze` to ensure complete absence of info, warning, or error messages. +- **Unit & Integration Tests**: Author comprehensive test coverage in `test/agent__test.dart` and `test/agent__integration_test.dart` verifying behavior via mock process runners and actual temp Git repositories. Verify success using the `run_tests` tool. diff --git a/tool/dart_hooks/bin/agent_skill_lint.dart b/tool/dart_hooks/bin/agent_skill_lint.dart old mode 100644 new mode 100755 diff --git a/tool/dart_hooks/test/bin_executable_test.dart b/tool/dart_hooks/test/bin_executable_test.dart new file mode 100644 index 0000000..d4925f3 --- /dev/null +++ b/tool/dart_hooks/test/bin_executable_test.dart @@ -0,0 +1,38 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// 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:io'; +import 'package:test/test.dart'; + +void main() { + test('all .dart files in bin/ are executable', () { + // Locate the bin/ directory reliably. + var binDir = Directory('bin'); + if (!binDir.existsSync()) { + binDir = Directory('../bin'); + } + expect(binDir.existsSync(), isTrue, reason: 'bin/ directory should exist'); + + final List dartFiles = binDir + .listSync() + .whereType() + .where((File file) => file.path.endsWith('.dart')) + .toList(); + + expect(dartFiles, isNotEmpty, reason: 'Should find .dart files in bin/'); + + for (final file in dartFiles) { + final FileStat stat = file.statSync(); + final String modeString = stat.modeString(); + // modeString is formatted like 'rwxr-xr-x'. + // Index 2 corresponds to the owner's executable bit. + final bool isExecutable = modeString.length >= 3 && modeString[2] == 'x'; + expect( + isExecutable, + isTrue, + reason: '${file.path} is not marked executable (mode: $modeString)', + ); + } + }); +}