From e4c007943df63d523614fe6ebbeb70d39fd678ba Mon Sep 17 00:00:00 2001 From: nickshorebird Date: Tue, 12 May 2026 14:07:01 -0400 Subject: [PATCH] feat(shorebird_ci): smoother first-run experience (0.2.0) (#3757) --- packages/shorebird_ci/CHANGELOG.md | 18 +++ packages/shorebird_ci/README.md | 35 ++++- .../commands/affected_packages_command.dart | 37 ++++- .../lib/src/commands/generate_command.dart | 91 ++++++++++-- .../lib/src/commands/repo_root_option.dart | 30 +++- packages/shorebird_ci/pubspec.yaml | 2 +- .../test/affected_packages_command_test.dart | 78 +++++++++++ .../test/generate_command_test.dart | 129 +++++++++++++++++- 8 files changed, 391 insertions(+), 29 deletions(-) diff --git a/packages/shorebird_ci/CHANGELOG.md b/packages/shorebird_ci/CHANGELOG.md index 08378c69..38ba06b1 100644 --- a/packages/shorebird_ci/CHANGELOG.md +++ b/packages/shorebird_ci/CHANGELOG.md @@ -1,3 +1,21 @@ + + +# 0.2.0 + +- Generated dynamic workflow now pins `fetch-depth: 0` on the setup-job checkout. Without it, `affected_packages` fails on every PR because the default shallow checkout doesn't include `origin/main`. The static main workflow gets the same fix so dorny/paths-filter can diff on push events. +- Generated workflow now includes a `workflow_dispatch:` trigger and bypasses the affected-packages diff when triggered manually, so first-push (no diff base) and "force a full re-check" scenarios actually run CI against all packages instead of producing a green-but-empty run. +- Generated workflow emits a GitHub notice when the diff yields no affected packages, pointing users at the manual **Run workflow** button so a skipped matrix isn't mistaken for a full pass. +- Bumped static action pins to `actions/checkout@v6` so `--no-update-actions` doesn't ship a Node.js 20 deprecation warning. +- `shorebird_ci generate` now bumps action versions to current latest automatically after writing the workflow files. Pass `--no-update-actions` to skip the network call. +- `shorebird_ci affected_packages` prints a friendly error instead of a Dart stack trace when run outside a git repository or against an unreachable base ref. +- `--repo-root` defaults to `git rev-parse --show-toplevel` when not specified, matching git's own behavior. Pass `--repo-root` explicitly to override. +- README: remove stale "not yet on pub.dev" note; document why subpackages are covered both as standalone matrix jobs and inside the root Flutter job. +- Add `executables:` declaration so `dart pub global activate shorebird_ci` creates a `shorebird_ci` shim on `PATH`. Previously users had to invoke via `dart pub global run shorebird_ci:shorebird_ci`. (rolled in from the unreleased 0.1.1) + +# 0.1.1 + +- Add `executables:` declaration so `dart pub global activate shorebird_ci` creates a `shorebird_ci` shim on `PATH`. Previously users had to invoke via `dart pub global run shorebird_ci:shorebird_ci`. + # 0.1.0 - Initial release. diff --git a/packages/shorebird_ci/README.md b/packages/shorebird_ci/README.md index 2a9ea65a..8e9fa633 100644 --- a/packages/shorebird_ci/README.md +++ b/packages/shorebird_ci/README.md @@ -6,13 +6,11 @@ Designed to be used by both humans and AI agents. ## Install -Once published to pub.dev: - ```sh dart pub global activate shorebird_ci ``` -From a local checkout (current state — not yet on pub.dev): +Or from a local checkout: ```sh dart pub global activate --source path packages/shorebird_ci @@ -60,6 +58,30 @@ Plus a CSpell job if a cspell config file exists. Adding or removing packages requires no workflow changes — the setup job discovers them at runtime. +### Manual runs and the empty-diff case + +The workflow includes a `workflow_dispatch:` trigger so you can launch +a run from the **Run workflow** button in the Actions tab. Manual runs +bypass the affected-packages diff and execute CI against every +package, which is what you want when: + +- You just pushed an initial commit to `main` and the diff vs. + `origin/main` is empty. +- You want to force a full re-check after editing CI configuration. +- Something looks off and you want a baseline green run. + +For normal `push: main` events where the diff is empty, setup emits a +GitHub notice pointing at the manual button so a green-but-skipped run +isn't confused for a full pass. + +### `--no-update-actions` + +`generate` auto-bumps action pins by querying GitHub for current +latest majors. `--no-update-actions` skips that network call and +leaves the static pins in the template as-is. Use it in offline +environments. Once you push, Dependabot picks up bumps on its weekly +schedule. + ### `--style static` (advanced) `generate --style static` emits a pre-computed dorny `filters:` block, @@ -77,6 +99,13 @@ it's what you pay today. For most repos it's noise. If you have a high-volume monorepo where most PRs don't touch Dart, static lets the workflow skip entirely at the trigger level. +### A note on subpackage double-coverage + +Subpackages of a Flutter root get CI'd twice: once in their own +matrix job, once inside the root's job. Intentional. The root needs +them for `pub get`, and the standalone job gives focused per-package +pass/fail. Cost is a duplicate analyze/test on affected PRs. + ## For AI agents This tool handles the **deterministic parts** (package discovery, dep diff --git a/packages/shorebird_ci/lib/src/commands/affected_packages_command.dart b/packages/shorebird_ci/lib/src/commands/affected_packages_command.dart index 8489df45..c7176195 100644 --- a/packages/shorebird_ci/lib/src/commands/affected_packages_command.dart +++ b/packages/shorebird_ci/lib/src/commands/affected_packages_command.dart @@ -56,13 +56,19 @@ class AffectedPackagesCommand extends Command with RepoRootOption { @override Future run() async { - final result = affectedPackagesMetadata( - repoRoot: Directory(repoRoot), - baseRef: argResults!['base'] as String, - headRef: argResults!['head'] as String, - sdkFilter: argResults!['sdk'] as String?, - all: argResults!.flag('all'), - ); + final List> result; + try { + result = affectedPackagesMetadata( + repoRoot: Directory(repoRoot), + baseRef: argResults!['base'] as String, + headRef: argResults!['head'] as String, + sdkFilter: argResults!['sdk'] as String?, + all: argResults!.flag('all'), + ); + } on ProcessException catch (e) { + stderr.writeln(_friendlyGitError(e)); + return 1; + } if (result.isEmpty) { // Distinguish "no packages in repo" from "no affected packages". @@ -74,4 +80,21 @@ class AffectedPackagesCommand extends Command with RepoRootOption { stdout.writeln(jsonEncode(result)); return 0; } + + String _friendlyGitError(ProcessException e) { + final message = e.message.toLowerCase(); + if (message.contains('not a git repository')) { + return 'Not in a git repository. Run from inside a git checkout, or ' + 'pass --repo-root to a git repo.'; + } + if (message.contains('could not access') || + message.contains('bad revision') || + message.contains('unknown revision')) { + final base = argResults!['base'] as String; + return 'Base ref "$base" not found. Pass --base with a ref ' + 'that exists locally, or fetch the missing ref ' + '(e.g. `git fetch origin main`).'; + } + return 'git failed: ${e.message.trim()}'; // coverage:ignore-line + } } diff --git a/packages/shorebird_ci/lib/src/commands/generate_command.dart b/packages/shorebird_ci/lib/src/commands/generate_command.dart index 75c15120..ffb4b303 100644 --- a/packages/shorebird_ci/lib/src/commands/generate_command.dart +++ b/packages/shorebird_ci/lib/src/commands/generate_command.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:path/path.dart' as p; +import 'package:shorebird_ci/src/action_versions.dart'; import 'package:shorebird_ci/src/commands/repo_root_option.dart'; import 'package:shorebird_ci/src/dependency_resolver.dart'; import 'package:shorebird_ci/src/flutter_version_resolver.dart'; @@ -11,8 +12,11 @@ import 'package:shorebird_ci/src/repository_description.dart'; /// Generates a GitHub Actions CI workflow for a Dart/Flutter repository. class GenerateCommand extends Command with RepoRootOption { - /// Creates a [GenerateCommand]. - GenerateCommand() { + /// Creates a [GenerateCommand]. The optional [resolveLatestMajor] is + /// exposed for tests so they don't have to hit the live GitHub API + /// when verifying the `--update-actions` path. + GenerateCommand({LatestMajorResolver? resolveLatestMajor}) + : _resolveLatestMajor = resolveLatestMajor { addRepoRootOption(); argParser ..addOption( @@ -32,9 +36,20 @@ class GenerateCommand extends Command with RepoRootOption { help: 'Print the generated workflow to stdout ' 'instead of writing to a file.', + ) + ..addFlag( + 'no-update-actions', + help: + 'Skip the post-generate step that queries GitHub for current ' + 'action versions and bumps pins in place. By default, generate ' + 'auto-bumps action versions to current latest. Pass this flag ' + 'in offline environments.', + negatable: false, ); } + final LatestMajorResolver? _resolveLatestMajor; + @override String get name => 'generate'; @@ -88,9 +103,31 @@ class GenerateCommand extends Command with RepoRootOption { _ensureDependabotConfig(repoRoot); + if (!argResults!.flag('no-update-actions')) { + await _bumpActionVersions( + files: files.keys.map((rel) => File(p.join(repoRoot, rel))), + ); + } + return 0; } + /// Rewrites each file in place w/ latest-major action pins. Best-effort: + /// network failures leave the file untouched (handled by + /// [updateActionVersions], which returns null per-action on lookup + /// failure). + Future _bumpActionVersions({required Iterable files}) async { + stderr.writeln('Updating action versions...'); + for (final file in files) { + final before = file.readAsStringSync(); + final after = await updateActionVersions( + before, + resolveLatestMajor: _resolveLatestMajor, + ); + if (before != after) file.writeAsStringSync(after); + } + } + /// Creates `.github/dependabot.yml` with a `github-actions` ecosystem /// entry if the file doesn't already exist. void _ensureDependabotConfig(String repoRoot) { @@ -204,7 +241,10 @@ jobs: // a changed `path:` dependency would silently go uncovered. buffer.write(''' steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 + with: + # Full history so dorny/paths-filter can diff on push events. + fetch-depth: 0 - uses: dart-lang/setup-dart@v1 - run: dart pub global activate shorebird_ci - name: Verify CI coverage @@ -324,7 +364,7 @@ jobs: ci: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: submodules: recursive - uses: dart-lang/setup-dart@v1 @@ -397,7 +437,7 @@ jobs: ci: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: submodules: recursive - name: Setup Flutter @@ -454,6 +494,10 @@ on: push: branches: - main + # Manual dispatch runs CI against all packages, bypassing the + # affected-packages diff. Useful on the first push to a new repo (where + # there's no diff base) or when you want to force a full re-check. + workflow_dispatch: jobs: '''); @@ -509,11 +553,15 @@ jobs: ); } - buffer.write(''' + buffer.write(r''' steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: submodules: recursive + # affected_packages diffs against origin/main, which isn't in + # the default shallow checkout. Full history is needed for the + # diff to resolve. + fetch-depth: 0 - uses: dart-lang/setup-dart@v1 - run: dart pub global activate shorebird_ci # Verify first so we fail fast if CI coverage is broken and @@ -522,20 +570,39 @@ jobs: run: shorebird_ci verify - id: affected run: | + if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then + echo "::notice::Manual dispatch: running CI against all packages." + EXTRA_ARGS=--all + else + EXTRA_ARGS= + fi '''); if (hasDart) { buffer.write(r''' - DART=$(shorebird_ci affected_packages --sdk dart) + DART=$(shorebird_ci affected_packages --sdk dart $EXTRA_ARGS) echo "dart_packages=$DART" >> $GITHUB_OUTPUT '''); } if (hasFlutter) { buffer.write(r''' - FLUTTER=$(shorebird_ci affected_packages --sdk flutter) + FLUTTER=$(shorebird_ci affected_packages --sdk flutter $EXTRA_ARGS) echo "flutter_packages=$FLUTTER" >> $GITHUB_OUTPUT '''); } - buffer.writeln(); + // Friendly notice when the diff path yields nothing, e.g. on a push + // to main where HEAD already equals origin/main. Points users at the + // manual-dispatch button so the green check has context. + final emptyChecks = [ + if (hasDart) r'"$DART" == "[]"', + if (hasFlutter) r'"$FLUTTER" == "[]"', + ].join(' && '); + buffer + ..write(''' + if [[ "\${{ github.event_name }}" != "workflow_dispatch" ]] && [[ $emptyChecks ]]; then + echo "::notice::No affected packages in diff vs origin/main. Click 'Run workflow' in the Actions tab to run CI against all packages." + fi +''') + ..writeln(); } void _writeCiJob( @@ -584,7 +651,7 @@ jobs: run: working-directory: \${{ matrix.path }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: submodules: recursive '''; @@ -672,7 +739,7 @@ jobs: name: CSpell runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: submodules: recursive - uses: streetsidesoftware/cspell-action@v6 diff --git a/packages/shorebird_ci/lib/src/commands/repo_root_option.dart b/packages/shorebird_ci/lib/src/commands/repo_root_option.dart index 2c1ec506..6747d96a 100644 --- a/packages/shorebird_ci/lib/src/commands/repo_root_option.dart +++ b/packages/shorebird_ci/lib/src/commands/repo_root_option.dart @@ -1,17 +1,37 @@ +// cspell:words toplevel + +import 'dart:io'; + import 'package:args/command_runner.dart'; /// Adds a `--repo-root` option to a command and exposes it via -/// [repoRoot]. Default is `'.'` to match the convention used by every -/// other command in this package. +/// [repoRoot]. +/// +/// When `--repo-root` is not provided, the value is auto-discovered via +/// `git rev-parse --show-toplevel`, so commands work from any +/// subdirectory of a git checkout — same convention as `git` itself. +/// Falls back to `'.'` if git is unavailable or the cwd is not inside a +/// git repo. mixin RepoRootOption on Command { /// Call from the command's constructor to register the option. void addRepoRootOption() { argParser.addOption( 'repo-root', - help: 'Path to the repository root.', + help: + 'Path to the repository root. ' + 'Defaults to the output of `git rev-parse --show-toplevel`.', ); } - /// The resolved repo root, or `'.'` if not specified. - String get repoRoot => argResults!['repo-root'] as String? ?? '.'; + /// The resolved repo root. + String get repoRoot { + final explicit = argResults!['repo-root'] as String?; + if (explicit != null) return explicit; + final result = Process.runSync('git', ['rev-parse', '--show-toplevel']); + if (result.exitCode == 0) { + final stdout = (result.stdout as String).trim(); + if (stdout.isNotEmpty) return stdout; + } + return '.'; + } } diff --git a/packages/shorebird_ci/pubspec.yaml b/packages/shorebird_ci/pubspec.yaml index 7da73550..a159800d 100644 --- a/packages/shorebird_ci/pubspec.yaml +++ b/packages/shorebird_ci/pubspec.yaml @@ -3,7 +3,7 @@ description: >- CI tooling for Dart and Flutter monorepos. Generates GitHub Actions workflows, resolves affected packages via dependency graphs, and verifies path filters stay in sync. -version: 0.1.0 +version: 0.2.0 homepage: https://shorebird.dev repository: https://github.com/shorebirdtech/shorebird/tree/main/packages/shorebird_ci topics: [ci, github-actions, monorepo, shorebird] diff --git a/packages/shorebird_ci/test/affected_packages_command_test.dart b/packages/shorebird_ci/test/affected_packages_command_test.dart index 151d7713..b2ac9e9b 100644 --- a/packages/shorebird_ci/test/affected_packages_command_test.dart +++ b/packages/shorebird_ci/test/affected_packages_command_test.dart @@ -1,3 +1,5 @@ +// cspell:words toplevel autodiscovers autodiscovery + import 'dart:io'; import 'package:args/command_runner.dart'; @@ -172,4 +174,80 @@ void main() { test('command exposes a non-empty description', () { expect(AffectedPackagesCommand().description, isNotEmpty); }); + + test( + 'command exits 1 (not stack-traces) when run outside a git repo', + () async { + // tempDir is not a git repo here. Underlying helper throws + // ProcessException; the command should catch it and surface a + // friendly message instead of a Dart stack trace. + createPackage(tempDir, 'packages/foo', 'foo'); + + final runner = CommandRunner('test', 'test') + ..addCommand(AffectedPackagesCommand()); + final code = await runner.run([ + 'affected_packages', + '--repo-root', + tempDir.path, + ]); + expect(code, 1); + }, + ); + + test('command exits 1 when base ref is missing', () async { + // No origin/main in the temp git repo → git diff fails with + // "Could not access 'origin/main'". The command should report a + // friendly error and exit 1, not throw. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + + final runner = CommandRunner('test', 'test') + ..addCommand(AffectedPackagesCommand()); + final code = await runner.run([ + 'affected_packages', + '--repo-root', + tempDir.path, + ]); + expect(code, 1); + }); + + // The two tests below exercise the --repo-root autodiscovery path + // (`git rev-parse --show-toplevel`) by setting Directory.current. + // Save+restore in try/finally so subsequent tests in this file see + // the original cwd. Test isolation across files is fine because each + // test file runs in its own isolate w/ isolate-local cwd. + test('repoRoot autodiscovers via git rev-parse when not passed', () async { + initGitRepo(tempDir); + createPackage(tempDir, 'packages/foo', 'foo'); + + final original = Directory.current; + Directory.current = tempDir; + try { + final runner = CommandRunner('test', 'test') + ..addCommand(AffectedPackagesCommand()); + final code = await runner.run(['affected_packages', '--all']); + expect(code, 0); + } finally { + Directory.current = original; + } + }); + + test('repoRoot falls back to "." when not inside a git repo', () async { + // tempDir is intentionally NOT a git repo. The autodiscovery + // `git rev-parse` will exit non-zero → mixin returns '.', which + // resolves to tempDir (our cwd) — confirmed by the friendly-error + // exit 1 from running git diff in a non-git tree. + createPackage(tempDir, 'packages/foo', 'foo'); + + final original = Directory.current; + Directory.current = tempDir; + try { + final runner = CommandRunner('test', 'test') + ..addCommand(AffectedPackagesCommand()); + final code = await runner.run(['affected_packages']); + expect(code, 1); + } finally { + Directory.current = original; + } + }); } diff --git a/packages/shorebird_ci/test/generate_command_test.dart b/packages/shorebird_ci/test/generate_command_test.dart index 606b7f16..4fa96bc1 100644 --- a/packages/shorebird_ci/test/generate_command_test.dart +++ b/packages/shorebird_ci/test/generate_command_test.dart @@ -4,20 +4,28 @@ import 'package:args/command_runner.dart'; import 'package:path/path.dart' as p; import 'package:shorebird_ci/src/commands/commands.dart'; import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; import 'test_utils.dart'; /// Runs `generate` via the command runner. +/// +/// Auto-bumping action versions is disabled by default so tests don't +/// hit the live GitHub API. Pass `--update-actions` in [extra] (along +/// with an injected resolver via the [GenerateCommand] constructor) to +/// exercise that path. Future runGenerate( Directory repoRoot, { List extra = const [], + GenerateCommand? command, }) async { final runner = CommandRunner('test', 'test') - ..addCommand(GenerateCommand()); + ..addCommand(command ?? GenerateCommand()); return runner.run([ 'generate', '--repo-root', repoRoot.path, + '--no-update-actions', ...extra, ]); } @@ -314,6 +322,125 @@ void main() { ); }); }); + + group('--update-actions (auto-bump)', () { + test('rewrites action pins in place when enabled', () async { + // Inject a resolver that bumps every action to v99 so we can + // confirm the post-write step actually runs and mutates the file. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + + final runner = CommandRunner('test', 'test') + ..addCommand( + GenerateCommand(resolveLatestMajor: (_) async => 'v99'), + ); + final code = await runner.run([ + 'generate', + '--repo-root', + tempDir.path, + // No --no-update-actions here — auto-bump is the default. + ]); + + expect(code, 0); + final yaml = _readMain(tempDir); + // setup-dart and checkout were emitted at @v1/@v6 by the generator; + // resolver bumps them both to @v99. + expect(yaml, contains('actions/checkout@v99')); + expect(yaml, contains('dart-lang/setup-dart@v99')); + }); + + test('--no-update-actions skips the bump', () async { + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + + // Resolver would bump to v99 if called; --no-update-actions should + // prevent that, leaving the generator's @v6/@v1 pins untouched. + final runner = CommandRunner('test', 'test') + ..addCommand( + GenerateCommand(resolveLatestMajor: (_) async => 'v99'), + ); + final code = await runner.run([ + 'generate', + '--repo-root', + tempDir.path, + '--no-update-actions', + ]); + + expect(code, 0); + final yaml = _readMain(tempDir); + expect(yaml, isNot(contains('@v99'))); + expect(yaml, contains('actions/checkout@v6')); + }); + }); + + group('setup-job runner ergonomics', () { + test('dynamic workflow has workflow_dispatch trigger', () async { + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + await runGenerate(tempDir); + final yaml = _readMain(tempDir); + expect(yaml, contains('workflow_dispatch:')); + }); + + test('dynamic setup job pins fetch-depth: 0', () async { + // Without full history, affected_packages can't diff against + // origin/main on the runner. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + await runGenerate(tempDir); + final yaml = _readMain(tempDir); + expect(yaml, contains('fetch-depth: 0')); + }); + + test('dynamic setup passes --all on workflow_dispatch', () async { + // Manual dispatch should bypass the diff and run every package. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + await runGenerate(tempDir); + final yaml = _readMain(tempDir); + expect(yaml, contains('EXTRA_ARGS=--all')); + expect(yaml, contains(r'affected_packages --sdk dart $EXTRA_ARGS')); + }); + + test('dynamic setup logs a hint when diff yields no packages', () async { + // On push to main, HEAD == origin/main and the diff is empty. + // The workflow should tell the user about the manual button. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + await runGenerate(tempDir); + final yaml = _readMain(tempDir); + expect(yaml, contains("Click 'Run workflow' in the Actions tab")); + }); + + test('generated dynamic workflow is valid YAML', () async { + // Regression guard: a bash `\` continuation that drops to column 0 + // inside a `run: |` block silently breaks the YAML literal and + // the runner rejects the file. loadYaml catches this locally. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + await runGenerate(tempDir); + final yaml = _readMain(tempDir); + expect(() => loadYaml(yaml), returnsNormally); + }); + + test('static main yaml pins fetch-depth: 0', () async { + // dorny/paths-filter on push events needs full history. + createPackage(tempDir, 'packages/foo', 'foo'); + initGitRepo(tempDir); + final runner = CommandRunner('test', 'test') + ..addCommand(GenerateCommand(resolveLatestMajor: (_) async => null)); + await runner.run([ + 'generate', + '--repo-root', + tempDir.path, + '--style', + 'static', + '--no-update-actions', + ]); + final yaml = _readMain(tempDir); + expect(yaml, contains('fetch-depth: 0')); + }); + }); } String _readMain(Directory repoRoot) {