From 643d1707a49b967d48f8965df1141954abb5b494 Mon Sep 17 00:00:00 2001 From: Reid-Agent <269567208+reidbaker-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:41:01 -0400 Subject: [PATCH 1/5] Refactor internal return types and cognitive complexity - Eliminate named record return types in _buildContext and _loadIgnores in favor of standard Future single-value returns - Audit and document _parse* methods in config_parser.dart and helper methods in test suites - Extract top-level test helpers _createMockRelease and _runInstallScriptTest in install_script_test.dart to lower main function cognitive complexity score - Update definition-of-done skill fail-threshold to 20 --- .../skills/definition-of-done/SKILL.md | 2 +- .../lib/src/config_parser.dart | 244 +++++++++------ .../lib/src/validation_session.dart | 284 +++++++++++------- tool/dart_skills_lint/lib/src/validator.dart | 139 +++++---- .../test/install_script_test.dart | 282 +++++++++-------- .../test/rules_md_consistency_test.dart | 89 +++--- 6 files changed, 622 insertions(+), 418 deletions(-) diff --git a/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md b/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md index e9d4c6d8..bf148eba 100644 --- a/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md +++ b/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md @@ -15,7 +15,7 @@ Before stating that a task is complete, you MUST execute and pass the following 1. **Format**: Run `dart format .` to format files, or `dart format --output=none --set-exit-if-changed .` to check without modifying. Ensure all files are formatted correctly. 2. **Analysis**: Run `dart analyze --fatal-infos` and ensure there are zero issues (including info-level issues). -3. **Metrics**: Run `dart run cognitive_complexity --fail-threshold 48 lib test` and ensure there are zero issues. This checks for cognitive complexity. +3. **Metrics**: Run `dart run cognitive_complexity --fail-threshold 20 lib test` and ensure there are zero issues. This checks for cognitive complexity. 4. **Tests**: Run `dart test` and ensure all tests pass successfully. 5. **Skills**: If any skill files were modified, run `dart run dart_skills_lint -d .agents/skills` to ensure they are valid. 6. **Changelog**: If the task introduces user-facing CLI flags, package API changes, bug fixes, or user-facing behavioral changes, update `CHANGELOG.md`. diff --git a/tool/dart_skills_lint/lib/src/config_parser.dart b/tool/dart_skills_lint/lib/src/config_parser.dart index 435ca35d..e0a83e35 100644 --- a/tool/dart_skills_lint/lib/src/config_parser.dart +++ b/tool/dart_skills_lint/lib/src/config_parser.dart @@ -107,7 +107,7 @@ class ConfigParser { } } - /// Parses the baseline rules configuration under the top-level `rules` key. + /// Parses the project-wide default rule configurations from the top-level `rules` map. /// /// The settings parsed here serve as the global defaults that apply to all /// validated skills in the project. Any target-specific settings defined @@ -128,7 +128,7 @@ class ConfigParser { return const {}; } - /// Parses a map of rules to their respective severity and parameter configurations. + /// Iterates a YAML rules map and converts each entry into a [RuleConfigPatch]. /// /// Validates that parameter keys and value types match their definitions in the registry, /// appending any validation errors to [parsingErrors] labeled by [contextLabel]. @@ -147,121 +147,179 @@ class ConfigParser { final checkMatches = RuleRegistry.allChecks.where((c) => c.name == ruleName); final CheckType? check = checkMatches.isEmpty ? null : checkMatches.first; - if (value is YamlMap) { - final severity = value.containsKey(_severityKey) - ? _parseSeverity(value[_severityKey]?.toString() ?? '') - : null; - - final parameters = {}; - for (final paramKey in value.keys) { - final paramName = paramKey.toString(); - if (paramName == _severityKey) { - continue; - } - parameters[paramName] = value[paramKey]; - } + ruleConfigs[ruleName] = _parseRuleConfigPatch(value, check, parsingErrors, contextLabel); + } - final customParams = parameters.isNotEmpty ? CustomRuleParameters(parameters) : null; - ruleConfigs[ruleName] = RuleConfigPatch(severity: severity, parameters: customParams); + return ruleConfigs; + } - if (customParams != null && check != null) { - final errors = check.validateParameters(customParams); - for (final error in errors) { - parsingErrors.add('$contextLabel: $error'); - } - } - } else { - final severity = _parseSeverity(value?.toString() ?? ''); - ruleConfigs[ruleName] = RuleConfigPatch(severity: severity); + /// Parses a single rule's configuration value into a [RuleConfigPatch]. + /// + /// Supports simple scalar severity declarations (e.g., `rule-name: error`) as + /// well as map declarations containing custom parameter overrides and severity + /// settings (e.g., `rule-name: { severity: error, param: value }`). Validates + /// any custom parameters against [check], appending schema validation errors + /// to [parsingErrors] labeled with [contextLabel]. + static RuleConfigPatch _parseRuleConfigPatch( + Object? value, + CheckType? check, + List parsingErrors, + String contextLabel, + ) { + if (value is! YamlMap) { + final severity = _parseSeverity(value?.toString() ?? ''); + return RuleConfigPatch(severity: severity); + } + + final severity = value.containsKey(_severityKey) + ? _parseSeverity(value[_severityKey]?.toString() ?? '') + : null; + + final parameters = {}; + for (final paramKey in value.keys) { + final paramName = paramKey.toString(); + if (paramName != _severityKey) { + parameters[paramName] = value[paramKey]; } } - return ruleConfigs; + final customParams = parameters.isNotEmpty ? CustomRuleParameters(parameters) : null; + + if (customParams != null && check != null) { + final errors = check.validateParameters(customParams); + for (final error in errors) { + parsingErrors.add('$contextLabel: $error'); + } + } + + return RuleConfigPatch(severity: severity, parameters: customParams); } - /// Parses a list of targets (directories or individual skills) from the configuration. - /// Validates keys for each entry and resolves path-specific rule overrides. - /// Appends any parsing errors to `parsingErrors`. + /// Iterates a top-level YAML target list (`directories` or `individual_skills`) + /// and parses each element into a [LintTargetConfig]. /// - /// Each entry is parsed defensively: a bad `path:` / `ignore_file:` / - /// `rules:` type emits a parsingErrors entry naming the offending field - /// and the entry is skipped, but later entries in the same list - /// still parse normally. + /// Delegates validation of an individual list element to [_parseTargetEntry]. + /// Returns an empty list if [configKey] is omitted or not a list. static List _parseConfigList( YamlMap toolConfig, String configKey, List parsingErrors, ) { + if (!toolConfig.containsKey(configKey)) { + return const []; + } + final items = toolConfig[configKey]; + if (items is! YamlList) { + return const []; + } + final entryLabelCap = configKey == _directoriesKey ? 'Directory entry' : 'Individual skill entry'; final entryLabelLower = configKey == _directoriesKey ? 'directory entry' : 'individual skill entry'; + final configs = []; - if (toolConfig.containsKey(configKey)) { - final items = toolConfig[configKey]; - if (items is YamlList) { - for (final dir in items) { - if (dir is! YamlMap || !dir.containsKey(_pathKey)) { - continue; - } - - final pathValue = dir[_pathKey]; - if (pathValue is! String) { - parsingErrors.add( - '$entryLabelCap "$_pathKey" must be a string; got "$pathValue" ' - '(${pathValue.runtimeType}). Skipping entry.', - ); - continue; - } - final String path = pathValue; - - for (final key in dir.keys) { - if (!_allowedDirectoryKeys.contains(key.toString())) { - parsingErrors.add('Unrecognized key "$key" in $entryLabelLower for "$path".'); - } - } - - Map ruleConfigs = const {}; - if (dir.containsKey(_rulesKey)) { - final localRules = dir[_rulesKey]; - if (localRules is YamlMap) { - ruleConfigs = _parseRulesMap( - localRules, - parsingErrors, - '$entryLabelCap rules for "$path"', - ); - } else { - parsingErrors.add( - '$entryLabelCap "$_rulesKey" for "$path" must be a map; ' - 'got "$localRules" (${localRules.runtimeType}). Ignoring local rules.', - ); - } - } - - String? ignoreFile; - if (dir.containsKey(_ignoreFileKey)) { - final ignoreFileValue = dir[_ignoreFileKey]; - if (ignoreFileValue is String) { - ignoreFile = ignoreFileValue; - } else if (ignoreFileValue != null) { - parsingErrors.add( - '$entryLabelCap "$_ignoreFileKey" for "$path" must be a string; ' - 'got "$ignoreFileValue" (${ignoreFileValue.runtimeType}). ' - 'Falling back to the default ignore file.', - ); - } - } - - configs.add( - LintTargetConfig(path: path, ruleConfigs: ruleConfigs, ignoreFile: ignoreFile), - ); - } + for (final dir in items) { + if (dir is! YamlMap || !dir.containsKey(_pathKey)) { + continue; + } + final config = _parseTargetEntry(dir, entryLabelCap, entryLabelLower, parsingErrors); + if (config != null) { + configs.add(config); } } return configs; } + + /// Parses a single dictionary element from a target list (`directories` or `individual_skills`). + /// + /// Validates the `path` string and checks for unrecognized keys. Delegates + /// parsing of sub-keys to [_parseLocalRulesForTarget] (`rules`) and + /// [_parseIgnoreFileForTarget] (`ignore_file`). Returns `null` if `path` is + /// invalid or missing. + static LintTargetConfig? _parseTargetEntry( + YamlMap dir, + String entryLabelCap, + String entryLabelLower, + List parsingErrors, + ) { + final pathValue = dir[_pathKey]; + if (pathValue is! String) { + parsingErrors.add( + '$entryLabelCap "$_pathKey" must be a string; got "$pathValue" ' + '(${pathValue.runtimeType}). Skipping entry.', + ); + return null; + } + final String path = pathValue; + + for (final key in dir.keys) { + if (!_allowedDirectoryKeys.contains(key.toString())) { + parsingErrors.add('Unrecognized key "$key" in $entryLabelLower for "$path".'); + } + } + + final ruleConfigs = _parseLocalRulesForTarget(dir, path, entryLabelCap, parsingErrors); + + final ignoreFile = _parseIgnoreFileForTarget(dir, path, entryLabelCap, parsingErrors); + + return LintTargetConfig(path: path, ruleConfigs: ruleConfigs, ignoreFile: ignoreFile); + } + + /// Parses path-specific rule overrides under a target entry's `rules` key. + /// + /// Unlike [_parseDefaultRules], which sets global baselines, configurations + /// parsed here apply only to skills within this specific target path. + /// Delegates to [_parseRulesMap]. + static Map _parseLocalRulesForTarget( + YamlMap dir, + String path, + String entryLabelCap, + List parsingErrors, + ) { + if (!dir.containsKey(_rulesKey)) { + return const {}; + } + final localRules = dir[_rulesKey]; + if (localRules is YamlMap) { + return _parseRulesMap(localRules, parsingErrors, '$entryLabelCap rules for "$path"'); + } + parsingErrors.add( + '$entryLabelCap "$_rulesKey" for "$path" must be a map; ' + 'got "$localRules" (${localRules.runtimeType}). Ignoring local rules.', + ); + return const {}; + } + + /// Parses the custom ignore file path under a target entry's `ignore_file` key. + /// + /// Returns `null` if omitted. If present but not a string, appends a type + /// error to [parsingErrors] and returns `null` to fall back to the default + /// ignore file. + static String? _parseIgnoreFileForTarget( + YamlMap dir, + String path, + String entryLabelCap, + List parsingErrors, + ) { + if (!dir.containsKey(_ignoreFileKey)) { + return null; + } + final ignoreFileValue = dir[_ignoreFileKey]; + if (ignoreFileValue is String) { + return ignoreFileValue; + } + if (ignoreFileValue != null) { + parsingErrors.add( + '$entryLabelCap "$_ignoreFileKey" for "$path" must be a string; ' + 'got "$ignoreFileValue" (${ignoreFileValue.runtimeType}). ' + 'Falling back to the default ignore file.', + ); + } + return null; + } } /// Configuration for a specific directory containing skills, or an individual skill. diff --git a/tool/dart_skills_lint/lib/src/validation_session.dart b/tool/dart_skills_lint/lib/src/validation_session.dart index acde8e99..195e672c 100644 --- a/tool/dart_skills_lint/lib/src/validation_session.dart +++ b/tool/dart_skills_lint/lib/src/validation_session.dart @@ -155,11 +155,11 @@ class ValidationSession { final String? localIgnoreFile = resolveIgnoreFile(normalizedSkillPath); final validator = Validator(ruleConfigs: resolvedConfigs, customRules: customRules); - final ({SkillsIgnores ignores, String ignorePath}) loaded = await _loadIgnores( - localIgnoreFile, - skillDir, + final String ignorePath = _resolveIgnorePath(localIgnoreFile, skillDir); + final SkillsIgnores ignores = await _loadIgnores( + ignorePath, + isCustomIgnoreFile: localIgnoreFile != null, ); - final SkillsIgnores ignores = loaded.ignores; final String skillName = p.basename(skillDir.path); final List skillIgnores = ignores.skills[skillName] ?? []; @@ -171,7 +171,7 @@ class ValidationSession { ); if (generateBaseline) { - await _saveBaseline(loaded.ignorePath, ignores); + await _saveBaseline(ignorePath, ignores); } else { final String fullPath = p.absolute(skillDir.path); for (final ignore in skillIgnores) { @@ -230,52 +230,93 @@ class ValidationSession { final Map loadedIgnoresCache = {}; for (final entity in entities) { - if (entity is! Directory) { - continue; - } - if (p.basename(entity.path).startsWith('.')) { + if (entity is! Directory || p.basename(entity.path).startsWith('.')) { continue; } - final String normalizedSkillPath = p.normalize(entity.path); - final Map resolvedConfigs = resolveRuleConfigsForPath( - normalizedSkillPath, + final bool shouldContinue = await _processRootSkillEntity( + entity, + rootDir, + loadedIgnoresCache, ); - final String? localIgnoreFile = resolveIgnoreFile(normalizedSkillPath); - final validator = Validator(ruleConfigs: resolvedConfigs, customRules: customRules); + if (!shouldContinue) { + break; + } + } - final String ignorePath = localIgnoreFile != null - ? p.normalize(expandPath(localIgnoreFile)) - : p.join(rootDir.path, defaultIgnoreFileName); + await _finalizeIgnoresForRoot(loadedIgnoresCache, rootDir); - final SkillsIgnores ignores; - if (loadedIgnoresCache.containsKey(ignorePath)) { - ignores = loadedIgnoresCache[ignorePath]!; - } else { - final ({SkillsIgnores ignores, String ignorePath}) loaded = await _loadIgnores( - localIgnoreFile, - rootDir, - ); - ignores = loaded.ignores; - loadedIgnoresCache[ignorePath] = ignores; - } + return !(_anyFailed && fastFail); + } - _anySkillsValidated = true; - final ValidationResult finalResult = await _runValidationWorkflow( - skillDir: entity, - validator: validator, - ignores: ignores, - ); + /// Processes and validates a single skill directory ([entity]) located + /// immediately inside a skills root directory ([rootDir]). + /// + /// In this context, "root" refers to the container directory passed via + /// `--skills-directory` / `-d` (represented by [rootDir]), which holds one or + /// more child skill folders. [entity] is an individual skill folder within + /// that root container. + /// + /// Returns `true` if iteration over the remaining skills in [rootDir] should + /// continue, or `false` to abort early when [fastFail] is enabled and this + /// skill failed validation. + Future _processRootSkillEntity( + Directory entity, + Directory rootDir, + Map loadedIgnoresCache, + ) async { + final String normalizedSkillPath = p.normalize(entity.path); + final Map resolvedConfigs = resolveRuleConfigsForPath(normalizedSkillPath); + final String? localIgnoreFile = resolveIgnoreFile(normalizedSkillPath); + final validator = Validator(ruleConfigs: resolvedConfigs, customRules: customRules); - if (!finalResult.isValid) { - _anyFailed = true; - if (fastFail) { - break; - } + final SkillsIgnores ignores = await _getIgnoresForSkill( + localIgnoreFile, + normalizedSkillPath, + rootDir, + loadedIgnoresCache, + ); + + _anySkillsValidated = true; + final ValidationResult finalResult = await _runValidationWorkflow( + skillDir: entity, + validator: validator, + ignores: ignores, + ); + + if (!finalResult.isValid) { + _anyFailed = true; + if (fastFail) { + return false; } } + return true; + } + + Future _getIgnoresForSkill( + String? localIgnoreFile, + String normalizedSkillPath, + Directory rootDir, + Map loadedIgnoresCache, + ) async { + final String ignorePath = _resolveIgnorePath(localIgnoreFile, rootDir); - // Save baselines and report stale entries for each loaded ignore file + if (loadedIgnoresCache.containsKey(ignorePath)) { + return loadedIgnoresCache[ignorePath]!; + } + + final SkillsIgnores ignores = await _loadIgnores( + ignorePath, + isCustomIgnoreFile: localIgnoreFile != null, + ); + loadedIgnoresCache[ignorePath] = ignores; + return ignores; + } + + Future _finalizeIgnoresForRoot( + Map loadedIgnoresCache, + Directory rootDir, + ) async { for (final MapEntry entry in loadedIgnoresCache.entries) { final String ignorePath = entry.key; final SkillsIgnores ignores = entry.value; @@ -283,22 +324,24 @@ class ValidationSession { if (generateBaseline) { await _saveBaseline(ignorePath, ignores); } else { - for (final MapEntry> skillEntry in ignores.skills.entries) { - final String skillName = skillEntry.key; - for (final IgnoreEntry ignore in skillEntry.value) { - if (!ignore.used) { - final String fullPath = p.absolute(p.join(rootDir.path, skillName)); - _log.info( - "Stale ignore entry found for rule '${ignore.ruleId}' in skill " - "'$skillName' at '$fullPath'. Consider removing it.", - ); - } - } - } + _reportStaleIgnores(ignores, rootDir); } } + } - return !(_anyFailed && fastFail); + void _reportStaleIgnores(SkillsIgnores ignores, Directory rootDir) { + for (final MapEntry> skillEntry in ignores.skills.entries) { + final String skillName = skillEntry.key; + for (final IgnoreEntry ignore in skillEntry.value) { + if (!ignore.used) { + final String fullPath = p.absolute(p.join(rootDir.path, skillName)); + _log.info( + "Stale ignore entry found for rule '${ignore.ruleId}' in skill " + "'$skillName' at '$fullPath'. Consider removing it.", + ); + } + } + } } /// If no skills were validated across the whole run, emit appropriate @@ -393,32 +436,27 @@ class ValidationSession { return resolvedIgnoreFile; } - /// Loads the ignore JSON for a root, returning both the parsed - /// [SkillsIgnores] and the resolved on-disk path it came from (or where it - /// would be written). - /// - /// Returning the [SkillsIgnores] object (not just `.skills`) lets callers - /// mutate it in memory across all skills in a root and then save it once, - /// instead of doing a load+save round-trip per skill. - Future<({SkillsIgnores ignores, String ignorePath})> _loadIgnores( - String? localIgnoreFile, - Directory rootDir, - ) async { - final String ignorePath = localIgnoreFile != null + String _resolveIgnorePath(String? localIgnoreFile, Directory rootDir) { + return localIgnoreFile != null ? p.normalize(expandPath(localIgnoreFile)) : p.join(rootDir.path, defaultIgnoreFileName); + } + /// Loads the ignore JSON from [ignorePath], returning the parsed [SkillsIgnores]. + /// + /// If [isCustomIgnoreFile] is true and the file does not exist, generates an + /// empty baseline file on disk. + Future _loadIgnores(String ignorePath, {required bool isCustomIgnoreFile}) async { final file = File(ignorePath); if (file.existsSync()) { final storage = SkillsIgnoresStorage(); - final SkillsIgnores ignores = await storage.load(ignorePath); - return (ignores: ignores, ignorePath: ignorePath); + return storage.load(ignorePath); } // If a custom ignore file was specified but not found, create an empty one // so the user can start adding ignores to it. - if (localIgnoreFile != null) { + if (isCustomIgnoreFile) { _log.warning('File not found generating-baseline'); try { await file.writeAsString(jsonEncode({SkillsIgnores.skillsKey: {}})); @@ -427,7 +465,7 @@ class ValidationSession { } } - return (ignores: SkillsIgnores(skills: {}), ignorePath: ignorePath); + return SkillsIgnores(skills: {}); } void _applyIgnores(ValidationResult result, List ignores) { @@ -517,57 +555,91 @@ class ValidationSession { return result; } - final String skillName = p.basename(skillDir.path); final skillMdFile = File(p.join(skillDir.path, SkillContext.skillFileName)); if (!skillMdFile.existsSync()) { return result; } + final String fixedContent = await _runFixableRules( + context: context, + result: result, + validator: validator, + ); + + if (fixedContent == context.rawContent) { + return result; + } + + return _handleFixResult( + skillDir: skillDir, + skillMdFile: skillMdFile, + originalContent: context.rawContent, + currentContent: fixedContent, + validator: validator, + skillIgnores: skillIgnores, + fallbackResult: result, + ); + } + + /// Runs all fixable rules against [context.rawContent] sequentially and + /// returns the resulting content string. + Future _runFixableRules({ + required SkillContext context, + required ValidationResult result, + required Validator validator, + }) async { String currentContent = context.rawContent; - final originalContent = currentContent; - var modified = false; for (final SkillRule rule in validator.rules) { - if (rule is FixableRule) { - final bool hasErrors = result.validationErrors.any( - (e) => e.ruleId == rule.name && !e.isIgnored, + if (rule is! FixableRule) { + continue; + } + final bool hasErrors = result.validationErrors.any( + (e) => e.ruleId == rule.name && !e.isIgnored, + ); + if (!hasErrors) { + continue; + } + + try { + final String newContent = await rule.fix( + SkillContext.skillFileName, + currentContent, + context.directory, ); - if (hasErrors) { - try { - final String newContent = await rule.fix( - SkillContext.skillFileName, - currentContent, - context.directory, - ); - if (newContent != currentContent) { - currentContent = newContent; - modified = true; - } - } catch (e) { - _log.severe(" Failed to apply fix for rule '${rule.name}': $e"); - } - } + currentContent = newContent; + } catch (e) { + _log.severe(" Failed to apply fix for rule '${rule.name}': $e"); } } - if (modified) { - if (fixApply) { - await skillMdFile.writeAsString(currentContent); - if (!quiet) { - _log.info(' Applied fixes for $skillName'); - } - final ValidationResult newResult = await validator.validate(skillDir); - _applyIgnores(newResult, skillIgnores); - return newResult; - } else if (fix) { - if (!quiet) { - _log.info(' [Dry Run] Proposed changes for $skillName (SKILL.md):'); - _printDiff(originalContent, currentContent); - } + return currentContent; + } + + Future _handleFixResult({ + required Directory skillDir, + required File skillMdFile, + required String originalContent, + required String currentContent, + required Validator validator, + required List skillIgnores, + required ValidationResult fallbackResult, + }) async { + final String skillName = p.basename(skillDir.path); + if (fixApply) { + await skillMdFile.writeAsString(currentContent); + if (!quiet) { + _log.info(' Applied fixes for $skillName'); } + final ValidationResult newResult = await validator.validate(skillDir); + _applyIgnores(newResult, skillIgnores); + return newResult; } - - return result; + if (fix && !quiet) { + _log.info(' [Dry Run] Proposed changes for $skillName (SKILL.md):'); + _printDiff(originalContent, currentContent); + } + return fallbackResult; } /// Prints a simple line-by-line diff between [original] and [modified]. diff --git a/tool/dart_skills_lint/lib/src/validator.dart b/tool/dart_skills_lint/lib/src/validator.dart index 043f88ec..262b073a 100644 --- a/tool/dart_skills_lint/lib/src/validator.dart +++ b/tool/dart_skills_lint/lib/src/validator.dart @@ -88,83 +88,106 @@ class Validator { /// Scans the directory for `SKILL.md`, parses its YAML metadata, and validates /// constraints like name format and field lengths using registered rules. Future validate(Directory dir) async { - final validationErrors = []; final skillMdFile = File(p.join(dir.path, _skillFileName)); final bool skillMdExists = dir.existsSync() && skillMdFile.existsSync(); - var content = ''; - YamlMap? parsedYaml; - String? yamlParsingError; + final fatalErrors = []; + final SkillContext? context = await _buildContext(dir, skillMdFile, skillMdExists, fatalErrors); + if (context == null) { + return ValidationResult(validationErrors: fatalErrors); + } - if (skillMdExists) { - try { - content = await skillMdFile.readAsString(); - } on FileSystemException catch (e) { - validationErrors.add( - ValidationError( - ruleId: skillFileInaccessible, - file: skillMdFile.path, - message: 'Failed to read $_skillFileName: $e', - severity: _getSeverity(skillFileInaccessible, AnalysisSeverity.error), - ), - ); - return ValidationResult(validationErrors: validationErrors); - } catch (e) { - validationErrors.add( - ValidationError( - ruleId: unexpectedError, - file: skillMdFile.path, - message: 'Unexpected error reading $_skillFileName: $e', - severity: _getSeverity(unexpectedError, AnalysisSeverity.error), - ), - ); - return ValidationResult(validationErrors: validationErrors); + final validationErrors = []; + + for (final SkillRule rule in _rules) { + // If SKILL.md or the directory does not exist or is inaccessible, running content validation rules + // against empty or non-existent content produces redundant cascading errors. We run solely PathDoesNotExistRule + // to report the missing structure cleanly, skipping subsequent rules. + if (!skillMdExists && rule.name != PathDoesNotExistRule.ruleName) { + continue; } + final List errors = await rule.validate(context); + _checkSeverityWarnings(rule, errors); + validationErrors.addAll(errors); + } - try { - final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(content); - if (match != null) { - final String yamlStr = match.group(1)!; - final Object? doc = loadYaml(yamlStr); - if (doc is YamlMap) { - parsedYaml = doc; - } else { - yamlParsingError = 'YAML frontmatter is not a map'; - } + return ValidationResult(validationErrors: validationErrors, context: context); + } + + /// Reads the skill file content and parses its YAML frontmatter to build a [SkillContext]. + /// + /// Appends any disk-read exception to [fatalErrors] and returns `null` if + /// [skillMdFile] cannot be read from disk. + Future _buildContext( + Directory dir, + File skillMdFile, + bool skillMdExists, + List fatalErrors, + ) async { + if (!skillMdExists) { + return SkillContext(directory: dir, rawContent: ''); + } + + final String content; + try { + content = await skillMdFile.readAsString(); + } on FileSystemException catch (e) { + fatalErrors.add( + ValidationError( + ruleId: skillFileInaccessible, + file: skillMdFile.path, + message: 'Failed to read $_skillFileName: $e', + severity: _getSeverity(skillFileInaccessible, AnalysisSeverity.error), + ), + ); + return null; + } catch (e) { + fatalErrors.add( + ValidationError( + ruleId: unexpectedError, + file: skillMdFile.path, + message: 'Unexpected error reading $_skillFileName: $e', + severity: _getSeverity(unexpectedError, AnalysisSeverity.error), + ), + ); + return null; + } + + YamlMap? parsedYaml; + String? yamlParsingError; + try { + final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(content); + if (match == null) { + yamlParsingError = 'Missing YAML metadata in $_skillFileName'; + } else { + final String yamlStr = match.group(1)!; + final Object? doc = loadYaml(yamlStr); + if (doc is YamlMap) { + parsedYaml = doc; } else { - yamlParsingError = 'Missing YAML metadata in $_skillFileName'; + yamlParsingError = 'YAML frontmatter is not a map'; } - } catch (e) { - yamlParsingError = 'Failed to parse YAML: $e'; } + } catch (e) { + yamlParsingError = 'Failed to parse YAML: $e'; } - final context = SkillContext( + return SkillContext( directory: dir, rawContent: content, parsedYaml: parsedYaml, yamlParsingError: yamlParsingError, ); + } - for (final SkillRule rule in _rules) { - // If SKILL.md or the directory does not exist or is inaccessible, running content validation rules - // against empty or non-existent content produces redundant cascading errors. We run solely PathDoesNotExistRule - // to report the missing structure cleanly, skipping subsequent rules. - if (!skillMdExists && rule.name != PathDoesNotExistRule.ruleName) { - continue; - } - final List errors = await rule.validate(context); - for (final error in errors) { - if (error.severity != rule.severity) { - _log.warning( - 'Rule "${rule.name}" used severity ${error.severity} instead of defined ${rule.severity}.', - ); - } + void _checkSeverityWarnings(SkillRule rule, List errors) { + for (final error in errors) { + if (error.severity != rule.severity) { + _log.warning( + 'Rule "${rule.name}" used severity ${error.severity} instead of defined ${rule.severity}.', + ); } - validationErrors.addAll(errors); } - - return ValidationResult(validationErrors: validationErrors, context: context); } /// Compiles the final list of active rules for the validator. diff --git a/tool/dart_skills_lint/test/install_script_test.dart b/tool/dart_skills_lint/test/install_script_test.dart index 7375436e..7e955cf6 100644 --- a/tool/dart_skills_lint/test/install_script_test.dart +++ b/tool/dart_skills_lint/test/install_script_test.dart @@ -93,129 +93,12 @@ void main() { } }); - /// Simulates a packaged GitHub release asset by writing a dummy binary, - /// compressing it to a `.tar.gz` archive in the mock release directory, - /// and generating the corresponding `SHA256SUMS` checksum file. - /// - /// If [shouldCorruptHash] is true, the `SHA256SUMS` file will be written with - /// an invalid hash to test checksum verification failure paths. - Future createMockRelease({ - required String os, - required String arch, - required String binaryContent, - bool shouldCorruptHash = false, - }) async { - final target = '$os-$arch'; - final binaryName = 'dart_skills_lint-$target'; - final archiveName = 'dart_skills_lint-$target.tar.gz'; - - // Create dummy binary file - final dummyBin = File(p.join(tempDir.path, binaryName)); - await dummyBin.writeAsString(binaryContent); - final ProcessResult chmodBinResult = await Process.run('chmod', ['+x', dummyBin.path]); - expect( - chmodBinResult.exitCode, - 0, - reason: 'chmod failed for dummy binary: ${chmodBinResult.stderr}', - ); - - // Package it into tar.gz - final ProcessResult tarResult = await Process.run('tar', [ - '-czf', - p.join(mockReleaseDir.path, archiveName), - '-C', - tempDir.path, - binaryName, - ]); - expect(tarResult.exitCode, 0, reason: 'tar packaging failed: ${tarResult.stderr}'); - - // Get SHA256 sum - var hash = ''; - // TODO(reidbaker): Re-add CertUtil checksum verification for Windows hosts. https://github.com/flutter/agent-plugins/issues/164 - final ProcessResult shaProcess = await Process.run('shasum', [ - '-a', - '256', - p.join(mockReleaseDir.path, archiveName), - ]); - if (shaProcess.exitCode == 0) { - hash = shaProcess.stdout.toString().trim().split(' ')[0]; - } else { - final ProcessResult sha256Process = await Process.run('sha256sum', [ - p.join(mockReleaseDir.path, archiveName), - ]); - if (sha256Process.exitCode == 0) { - hash = sha256Process.stdout.toString().trim().split(' ')[0]; - } - } - - if (hash.isEmpty) { - throw StateError('Could not calculate SHA256 hash using shasum or sha256sum.'); - } - - final String finalHash = shouldCorruptHash ? _corruptedHash : hash; - - final sha256sums = File(p.join(mockReleaseDir.path, 'SHA256SUMS')); - await sha256sums.writeAsString('$finalHash $archiveName\n'); - } - - Future runInstallScriptTest({ - required String os, - required String arch, - required String mockUnameS, - required String mockUnameM, - required bool simulateLaunchFailure, - required int expectedExitCode, - required bool expectInstalled, - }) async { - const version = '0.4.0-test'; - final binaryContent = simulateLaunchFailure - ? '#!/usr/bin/env bash\nexit 1\n' - : '#!/usr/bin/env bash\necho "mock-cli-help"\n'; - - await createMockRelease(os: os, arch: arch, binaryContent: binaryContent); - - // TODO(reidbaker): Use Windows path separator (;) when running on Windows hosts. https://github.com/flutter/agent-plugins/issues/164 - final newPath = '${mockBinDir.path}:${Platform.environment['PATH']}'; - final String packageRoot = _getPackageRoot(); - final String scriptPath = p.join(packageRoot, 'scripts', 'install.sh'); - - final TestProcess process = await TestProcess.start( - 'bash', - [scriptPath], - environment: { - 'PATH': newPath, - 'MOCK_UNAME_S': mockUnameS, - 'MOCK_UNAME_M': mockUnameM, - 'MOCK_RELEASE_DIR': mockReleaseDir.path, - 'INSTALL_DIR': installDir.path, - 'VERSION': version, - }, - ); - - await process.shouldExit(expectedExitCode); - - final installedFile = File(p.join(installDir.path, 'dart_skills_lint')); - expect(installedFile.existsSync(), equals(expectInstalled)); - - if (expectInstalled && expectedExitCode == 0) { - if (simulateLaunchFailure) { - final List stdout = await process.stdout.rest.toList(); - expect( - stdout.any((line) => line.contains('launch check failed — likely Gatekeeper')), - isTrue, - ); - } else { - final ProcessResult runResult = await Process.run(installedFile.path, ['--help']); - expect(runResult.stdout.toString().trim(), equals('mock-cli-help')); - } - } else if (expectedExitCode == 1 && simulateLaunchFailure) { - final List stderr = await process.stderr.rest.toList(); - expect(stderr.any((line) => line.contains('failed to launch')), isTrue); - } - } - test('successful installation on macos-arm64', () async { - await runInstallScriptTest( + await _runInstallScriptTest( + tempDir: tempDir, + mockBinDir: mockBinDir, + mockReleaseDir: mockReleaseDir, + installDir: installDir, os: 'macos', arch: 'arm64', mockUnameS: 'Darwin', @@ -227,7 +110,11 @@ void main() { }); test('successful installation on linux-x64', () async { - await runInstallScriptTest( + await _runInstallScriptTest( + tempDir: tempDir, + mockBinDir: mockBinDir, + mockReleaseDir: mockReleaseDir, + installDir: installDir, os: 'linux', arch: 'x64', mockUnameS: 'Linux', @@ -239,7 +126,11 @@ void main() { }); test('fails on linux if installed binary fails launch check', () async { - await runInstallScriptTest( + await _runInstallScriptTest( + tempDir: tempDir, + mockBinDir: mockBinDir, + mockReleaseDir: mockReleaseDir, + installDir: installDir, os: 'linux', arch: 'x64', mockUnameS: 'Linux', @@ -251,7 +142,11 @@ void main() { }); test('succeeds on macos even if installed binary fails launch check', () async { - await runInstallScriptTest( + await _runInstallScriptTest( + tempDir: tempDir, + mockBinDir: mockBinDir, + mockReleaseDir: mockReleaseDir, + installDir: installDir, os: 'macos', arch: 'arm64', mockUnameS: 'Darwin', @@ -264,7 +159,9 @@ void main() { test('fails if checksum mismatch', () async { const version = '0.4.0-test'; - await createMockRelease( + await _createMockRelease( + tempDir: tempDir, + mockReleaseDir: mockReleaseDir, os: 'macos', arch: 'arm64', binaryContent: 'dummy', @@ -459,3 +356,136 @@ String _getPackageRoot() { } return currentPath; } + +/// Simulates a packaged GitHub release asset by writing a dummy binary, +/// compressing it to a `.tar.gz` archive in [mockReleaseDir], and generating +/// the corresponding `SHA256SUMS` checksum file. +/// +/// If [shouldCorruptHash] is true, the `SHA256SUMS` file will be written with +/// an invalid hash to test checksum verification failure paths. +Future _createMockRelease({ + required Directory tempDir, + required Directory mockReleaseDir, + required String os, + required String arch, + required String binaryContent, + bool shouldCorruptHash = false, +}) async { + final target = '$os-$arch'; + final binaryName = 'dart_skills_lint-$target'; + final archiveName = 'dart_skills_lint-$target.tar.gz'; + + // Create dummy binary file + final dummyBin = File(p.join(tempDir.path, binaryName)); + await dummyBin.writeAsString(binaryContent); + final ProcessResult chmodBinResult = await Process.run('chmod', ['+x', dummyBin.path]); + expect( + chmodBinResult.exitCode, + 0, + reason: 'chmod failed for dummy binary: ${chmodBinResult.stderr}', + ); + + // Package it into tar.gz + final ProcessResult tarResult = await Process.run('tar', [ + '-czf', + p.join(mockReleaseDir.path, archiveName), + '-C', + tempDir.path, + binaryName, + ]); + expect(tarResult.exitCode, 0, reason: 'tar packaging failed: ${tarResult.stderr}'); + + // Get SHA256 sum + var hash = ''; + // TODO(reidbaker): Re-add CertUtil checksum verification for Windows hosts. https://github.com/flutter/agent-plugins/issues/164 + final ProcessResult shaProcess = await Process.run('shasum', [ + '-a', + '256', + p.join(mockReleaseDir.path, archiveName), + ]); + if (shaProcess.exitCode == 0) { + hash = shaProcess.stdout.toString().trim().split(' ')[0]; + } else { + final ProcessResult sha256Process = await Process.run('sha256sum', [ + p.join(mockReleaseDir.path, archiveName), + ]); + if (sha256Process.exitCode == 0) { + hash = sha256Process.stdout.toString().trim().split(' ')[0]; + } + } + + if (hash.isEmpty) { + throw StateError('Could not calculate SHA256 hash using shasum or sha256sum.'); + } + + final String finalHash = shouldCorruptHash ? _corruptedHash : hash; + + final sha256sums = File(p.join(mockReleaseDir.path, 'SHA256SUMS')); + await sha256sums.writeAsString('$finalHash $archiveName\n'); +} + +Future _runInstallScriptTest({ + required Directory tempDir, + required Directory mockBinDir, + required Directory mockReleaseDir, + required Directory installDir, + required String os, + required String arch, + required String mockUnameS, + required String mockUnameM, + required bool simulateLaunchFailure, + required int expectedExitCode, + required bool expectInstalled, +}) async { + const version = '0.4.0-test'; + final binaryContent = simulateLaunchFailure + ? '#!/usr/bin/env bash\nexit 1\n' + : '#!/usr/bin/env bash\necho "mock-cli-help"\n'; + + await _createMockRelease( + tempDir: tempDir, + mockReleaseDir: mockReleaseDir, + os: os, + arch: arch, + binaryContent: binaryContent, + ); + + // TODO(reidbaker): Use Windows path separator (;) when running on Windows hosts. https://github.com/flutter/agent-plugins/issues/164 + final newPath = '${mockBinDir.path}:${Platform.environment['PATH']}'; + final String packageRoot = _getPackageRoot(); + final String scriptPath = p.join(packageRoot, 'scripts', 'install.sh'); + + final TestProcess process = await TestProcess.start( + 'bash', + [scriptPath], + environment: { + 'PATH': newPath, + 'MOCK_UNAME_S': mockUnameS, + 'MOCK_UNAME_M': mockUnameM, + 'MOCK_RELEASE_DIR': mockReleaseDir.path, + 'INSTALL_DIR': installDir.path, + 'VERSION': version, + }, + ); + + await process.shouldExit(expectedExitCode); + + final installedFile = File(p.join(installDir.path, 'dart_skills_lint')); + expect(installedFile.existsSync(), equals(expectInstalled)); + + if (expectInstalled && expectedExitCode == 0) { + if (simulateLaunchFailure) { + final List stdout = await process.stdout.rest.toList(); + expect( + stdout.any((line) => line.contains('launch check failed — likely Gatekeeper')), + isTrue, + ); + } else { + final ProcessResult runResult = await Process.run(installedFile.path, ['--help']); + expect(runResult.stdout.toString().trim(), equals('mock-cli-help')); + } + } else if (expectedExitCode == 1 && simulateLaunchFailure) { + final List stderr = await process.stderr.rest.toList(); + expect(stderr.any((line) => line.contains('failed to launch')), isTrue); + } +} diff --git a/tool/dart_skills_lint/test/rules_md_consistency_test.dart b/tool/dart_skills_lint/test/rules_md_consistency_test.dart index 51e0bc03..94b675aa 100644 --- a/tool/dart_skills_lint/test/rules_md_consistency_test.dart +++ b/tool/dart_skills_lint/test/rules_md_consistency_test.dart @@ -64,20 +64,7 @@ void main() { }); test('RULES.md "Default severity:" matches CheckType.defaultSeverity', () { - final List mismatches = []; - for (final MapEntry entry in docRules.entries) { - final String name = entry.key; - final CheckType? check = registryByName[name]; - if (check == null) { - continue; - } - if (entry.value.defaultSeverity != check.defaultSeverity) { - mismatches.add( - '$name: RULES.md says ${entry.value.defaultSeverity.name}, ' - 'registry says ${check.defaultSeverity.name}', - ); - } - } + final List mismatches = _findSeverityMismatches(docRules, registryByName); expect( mismatches, isEmpty, @@ -88,26 +75,7 @@ void main() { }); test('RULES.md "Fixable:" matches whether the rule implements FixableRule', () { - final List mismatches = []; - for (final MapEntry entry in docRules.entries) { - final String name = entry.key; - final CheckType? check = registryByName[name]; - if (check == null) { - continue; - } - final SkillRule? rule = RuleRegistry.createRule(name, check.defaultSeverity); - if (rule == null) { - mismatches.add('$name: RuleRegistry.createRule returned null'); - continue; - } - final actuallyFixable = rule is FixableRule; - if (entry.value.fixable != actuallyFixable) { - mismatches.add( - '$name: RULES.md says fixable=${entry.value.fixable}, ' - 'class is FixableRule=$actuallyFixable', - ); - } - } + final List mismatches = _findFixableMismatches(docRules, registryByName); expect( mismatches, isEmpty, @@ -119,6 +87,59 @@ void main() { }); } +/// Returns descriptive mismatch strings for rules whose documented +/// `Default severity:` in `RULES.md` differs from [CheckType.defaultSeverity]. +List _findSeverityMismatches( + Map docRules, + Map registryByName, +) { + final List mismatches = []; + for (final MapEntry entry in docRules.entries) { + final String name = entry.key; + final CheckType? check = registryByName[name]; + if (check == null) { + continue; + } + if (entry.value.defaultSeverity != check.defaultSeverity) { + mismatches.add( + '$name: RULES.md says ${entry.value.defaultSeverity.name}, ' + 'registry says ${check.defaultSeverity.name}', + ); + } + } + return mismatches; +} + +/// Returns descriptive mismatch strings for rules whose documented +/// `Fixable:` claim in `RULES.md` differs from whether the rule class +/// implements [FixableRule]. +List _findFixableMismatches( + Map docRules, + Map registryByName, +) { + final List mismatches = []; + for (final MapEntry entry in docRules.entries) { + final String name = entry.key; + final CheckType? check = registryByName[name]; + if (check == null) { + continue; + } + final SkillRule? rule = RuleRegistry.createRule(name, check.defaultSeverity); + if (rule == null) { + mismatches.add('$name: RuleRegistry.createRule returned null'); + continue; + } + final actuallyFixable = rule is FixableRule; + if (entry.value.fixable != actuallyFixable) { + mismatches.add( + '$name: RULES.md says fixable=${entry.value.fixable}, ' + 'class is FixableRule=$actuallyFixable', + ); + } + } + return mismatches; +} + class _DocRule { _DocRule({required this.defaultSeverity, required this.fixable}); From adeb4def711025c162114e76aff8bc28ba6c26ec Mon Sep 17 00:00:00 2001 From: Reid-Agent <269567208+reidbaker-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:55:05 -0400 Subject: [PATCH 2/5] Tighten CI cognitive complexity fail-threshold to 20 (#212) --- .github/workflows/dart_skills_lint_workflow.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dart_skills_lint_workflow.yaml b/.github/workflows/dart_skills_lint_workflow.yaml index c211fa82..1187d5c1 100644 --- a/.github/workflows/dart_skills_lint_workflow.yaml +++ b/.github/workflows/dart_skills_lint_workflow.yaml @@ -41,7 +41,7 @@ jobs: - run: dart analyze --fatal-infos - name: Run cognitive complexity check - run: dart run cognitive_complexity --fail-threshold 48 tool/dart_skills_lint/lib tool/dart_skills_lint/test + run: dart run cognitive_complexity --fail-threshold 20 tool/dart_skills_lint/lib tool/dart_skills_lint/test - run: dart test From 11cc7a27409627e3c830758f2e7b7244fda83fe8 Mon Sep 17 00:00:00 2001 From: Reid-Agent <269567208+reidbaker-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:50:59 -0400 Subject: [PATCH 3/5] Automate CI threshold assertion and delegate DOD skill to CI workflow (#212) --- .../skills/definition-of-done/SKILL.md | 2 +- .../test/recipe_drift_test.dart | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md b/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md index bf148eba..51783440 100644 --- a/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md +++ b/tool/dart_skills_lint/.agents/skills/definition-of-done/SKILL.md @@ -15,7 +15,7 @@ Before stating that a task is complete, you MUST execute and pass the following 1. **Format**: Run `dart format .` to format files, or `dart format --output=none --set-exit-if-changed .` to check without modifying. Ensure all files are formatted correctly. 2. **Analysis**: Run `dart analyze --fatal-infos` and ensure there are zero issues (including info-level issues). -3. **Metrics**: Run `dart run cognitive_complexity --fail-threshold 20 lib test` and ensure there are zero issues. This checks for cognitive complexity. +3. **Metrics**: Run `dart run cognitive_complexity --fail-threshold lib test`, where `` is the `--fail-threshold` value configured in `.github/workflows/dart_skills_lint_workflow.yaml`, and ensure there are zero issues. This checks for cognitive complexity. 4. **Tests**: Run `dart test` and ensure all tests pass successfully. 5. **Skills**: If any skill files were modified, run `dart run dart_skills_lint -d .agents/skills` to ensure they are valid. 6. **Changelog**: If the task introduces user-facing CLI flags, package API changes, bug fixes, or user-facing behavioral changes, update `CHANGELOG.md`. diff --git a/tool/dart_skills_lint/test/recipe_drift_test.dart b/tool/dart_skills_lint/test/recipe_drift_test.dart index 098492b6..86d2d752 100644 --- a/tool/dart_skills_lint/test/recipe_drift_test.dart +++ b/tool/dart_skills_lint/test/recipe_drift_test.dart @@ -107,6 +107,27 @@ void main() { await _runHookAgainst(hookBody, validFixture, expectZeroExit: true); await _runHookAgainst(hookBody, invalidFixture, expectZeroExit: false); }); + + test('CI workflow cognitive complexity fail-threshold does not exceed 20', () { + final File workflowFile = _getWorkflowFile(); + expect(workflowFile.existsSync(), isTrue, reason: 'CI workflow file missing'); + final String content = workflowFile.readAsStringSync(); + final regex = RegExp( + r'dart\s+run\s+cognitive_complexity\s+--fail-threshold\s+(\d+)\s+tool/dart_skills_lint/lib\s+tool/dart_skills_lint/test', + ); + final RegExpMatch? match = regex.firstMatch(content); + expect( + match, + isNotNull, + reason: 'CI workflow must run cognitive_complexity with --fail-threshold ', + ); + final int threshold = int.parse(match!.group(1)!); + expect( + threshold, + lessThanOrEqualTo(20), + reason: 'cognitive complexity fail-threshold in CI ($threshold) should not exceed 20', + ); + }); }, skip: Platform.isWindows ? 'recipe drift uses POSIX shell' : null); } @@ -241,3 +262,17 @@ class _RecipeBlock { final String language; final String body; } + +File _getWorkflowFile() { + Directory dir = Directory.current; + while (dir.path != '/' && dir.path.isNotEmpty) { + final workflowFile = File( + p.join(dir.path, '.github', 'workflows', 'dart_skills_lint_workflow.yaml'), + ); + if (workflowFile.existsSync()) { + return workflowFile; + } + dir = dir.parent; + } + return File(p.normalize(p.absolute('../../.github/workflows/dart_skills_lint_workflow.yaml'))); +} From 029edaa57befed904aacf098879498c8cd86839b Mon Sep 17 00:00:00 2001 From: Reid-Agent <269567208+reidbaker-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:55:26 -0400 Subject: [PATCH 4/5] Move CI workflow consistency test out of recipe drift into dedicated test suite (#212) --- .../test/recipe_drift_test.dart | 35 -------------- .../test/workflow_consistency_test.dart | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 35 deletions(-) create mode 100644 tool/dart_skills_lint/test/workflow_consistency_test.dart diff --git a/tool/dart_skills_lint/test/recipe_drift_test.dart b/tool/dart_skills_lint/test/recipe_drift_test.dart index 86d2d752..098492b6 100644 --- a/tool/dart_skills_lint/test/recipe_drift_test.dart +++ b/tool/dart_skills_lint/test/recipe_drift_test.dart @@ -107,27 +107,6 @@ void main() { await _runHookAgainst(hookBody, validFixture, expectZeroExit: true); await _runHookAgainst(hookBody, invalidFixture, expectZeroExit: false); }); - - test('CI workflow cognitive complexity fail-threshold does not exceed 20', () { - final File workflowFile = _getWorkflowFile(); - expect(workflowFile.existsSync(), isTrue, reason: 'CI workflow file missing'); - final String content = workflowFile.readAsStringSync(); - final regex = RegExp( - r'dart\s+run\s+cognitive_complexity\s+--fail-threshold\s+(\d+)\s+tool/dart_skills_lint/lib\s+tool/dart_skills_lint/test', - ); - final RegExpMatch? match = regex.firstMatch(content); - expect( - match, - isNotNull, - reason: 'CI workflow must run cognitive_complexity with --fail-threshold ', - ); - final int threshold = int.parse(match!.group(1)!); - expect( - threshold, - lessThanOrEqualTo(20), - reason: 'cognitive complexity fail-threshold in CI ($threshold) should not exceed 20', - ); - }); }, skip: Platform.isWindows ? 'recipe drift uses POSIX shell' : null); } @@ -262,17 +241,3 @@ class _RecipeBlock { final String language; final String body; } - -File _getWorkflowFile() { - Directory dir = Directory.current; - while (dir.path != '/' && dir.path.isNotEmpty) { - final workflowFile = File( - p.join(dir.path, '.github', 'workflows', 'dart_skills_lint_workflow.yaml'), - ); - if (workflowFile.existsSync()) { - return workflowFile; - } - dir = dir.parent; - } - return File(p.normalize(p.absolute('../../.github/workflows/dart_skills_lint_workflow.yaml'))); -} diff --git a/tool/dart_skills_lint/test/workflow_consistency_test.dart b/tool/dart_skills_lint/test/workflow_consistency_test.dart new file mode 100644 index 00000000..0a9ceec8 --- /dev/null +++ b/tool/dart_skills_lint/test/workflow_consistency_test.dart @@ -0,0 +1,47 @@ +// 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 'package:test/test.dart'; + +void main() { + group('CI workflow consistency', () { + test('CI workflow cognitive complexity fail-threshold does not exceed 20', () { + final File workflowFile = _getWorkflowFile(); + expect(workflowFile.existsSync(), isTrue, reason: 'CI workflow file missing'); + final String content = workflowFile.readAsStringSync(); + final regex = RegExp( + r'dart\s+run\s+cognitive_complexity\s+--fail-threshold\s+(\d+)\s+tool/dart_skills_lint/lib\s+tool/dart_skills_lint/test', + ); + final RegExpMatch? match = regex.firstMatch(content); + expect( + match, + isNotNull, + reason: 'CI workflow must run cognitive_complexity with --fail-threshold ', + ); + final int threshold = int.parse(match!.group(1)!); + expect( + threshold, + lessThanOrEqualTo(20), + reason: 'cognitive complexity fail-threshold in CI ($threshold) should not exceed 20', + ); + }); + }); +} + +File _getWorkflowFile() { + Directory dir = Directory.current; + while (dir.path != '/' && dir.path.isNotEmpty) { + final workflowFile = File( + p.join(dir.path, '.github', 'workflows', 'dart_skills_lint_workflow.yaml'), + ); + if (workflowFile.existsSync()) { + return workflowFile; + } + dir = dir.parent; + } + return File(p.normalize(p.absolute('../../.github/workflows/dart_skills_lint_workflow.yaml'))); +} From fbb08afcd655916e59772ac9f9171fa02b90ff0b Mon Sep 17 00:00:00 2001 From: Reid-Agent <269567208+reidbaker-agent@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:27:06 -0400 Subject: [PATCH 5/5] Remove unused normalizedSkillPath from _getIgnoresForSkill (#212) --- tool/dart_skills_lint/lib/src/validation_session.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/tool/dart_skills_lint/lib/src/validation_session.dart b/tool/dart_skills_lint/lib/src/validation_session.dart index 195e672c..b7a8f703 100644 --- a/tool/dart_skills_lint/lib/src/validation_session.dart +++ b/tool/dart_skills_lint/lib/src/validation_session.dart @@ -272,7 +272,6 @@ class ValidationSession { final SkillsIgnores ignores = await _getIgnoresForSkill( localIgnoreFile, - normalizedSkillPath, rootDir, loadedIgnoresCache, ); @@ -295,7 +294,6 @@ class ValidationSession { Future _getIgnoresForSkill( String? localIgnoreFile, - String normalizedSkillPath, Directory rootDir, Map loadedIgnoresCache, ) async {