chore: automate release train metadata sync#16
Conversation
|
To preview the documentation for this pull request, visit the following URL: docs.page/arenukvern/intentcall~16
|
📝 WalkthroughWalkthroughAdds ChangesRelease Train Metadata Sync
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tool/intentcall/test/publish_preflight_test.dart (2)
339-394: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the fixture from
allInternalPackages, not a hand-maintained subset.
syncReleaseTrainMetadata()rewrites floors for every entry inallInternalPackages, but this fixture only materializespublishablePackagesplusintentcall_gemma. If another internal-only package is added, these tests will fail for setup reasons instead of sync behavior.Possible follow-up
+ final publishable = release_train.publishablePackages.toSet(); - for (final packageName in release_train.publishablePackages) { + for (final packageName in release_train.allInternalPackages) { final deps = switch (packageName) { ... + 'intentcall_gemma' => + ' intentcall_core: ^0.5.0\n' + ' intentcall_schema: ^0.5.0\n' + ' intentcall_testing: ^0.5.0\n', _ => '', }; - _writePubspec(repo, packageName, version: '0.6.0', dependencies: deps); + _writePubspec( + repo, + packageName, + version: '0.6.0', + dependencies: deps, + publishToNone: !publishable.contains(packageName), + ); } - - _writePubspec(... 'intentcall_gemma' ...)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tool/intentcall/test/publish_preflight_test.dart` around lines 339 - 394, The release-train fixture in _createReleaseTrainFixture is only creating publishablePackages plus intentcall_gemma, but syncReleaseTrainMetadata() operates over allInternalPackages. Update the fixture setup to materialize every package from allInternalPackages (including internal-only ones) so the test coverage stays aligned with the metadata sync logic and won’t break when new internal packages are added.
194-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the publishable
version:rewrite too.This test proves floor and podspec updates, but it never makes a publishable package version stale, so the first
syncReleaseTrainMetadata()loop is untested. Seed one publishablepubspec.yamlat0.5.0and assert it becomes0.6.0after sync.Possible follow-up
- _writePubspec(repo, packageName, version: '0.6.0', dependencies: deps); + _writePubspec( + repo, + packageName, + version: packageName == 'intentcall_core' ? '0.5.0' : '0.6.0', + dependencies: deps, + );expect(syncCode, 0); expect(await release_train.runReleaseTrainCheck(repo), 0); + expect( + File( + p.join(repo.path, 'packages', 'intentcall_core', 'pubspec.yaml'), + ).readAsStringSync(), + contains('version: 0.6.0'), + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tool/intentcall/test/publish_preflight_test.dart` around lines 194 - 229, The release train sync test only covers dependency floor and podspec updates, so it misses the publishable version rewrite path in syncReleaseTrainMetadata(). Update the fixture setup in publish_preflight_test.dart to make one publishable package pubspec.yaml stale at 0.5.0, then extend the assertions after runReleaseTrainSync(repo) to verify that the version is rewritten to 0.6.0, using the existing release_train.runReleaseTrainSync and runReleaseTrainCheck flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-pr-sync-train.yml:
- Around line 12-14: The workflow permissions are broader than needed because
the release PR sync job only pushes commits back to the branch, so the
`pull-requests: write` scope should be removed. Update the permissions block in
the release sync workflow to keep only the `contents: write` access, and leave
the rest of the job unchanged.
In `@justfile`:
- Around line 53-58: The sync-release-train targets still invoke the intentcall
entrypoint through dart run, which can depend on stale pub resolution; update
both sync-release-train and sync-release-train-version in the justfile to call
the dedicated release_train.dart sync script directly with dart
tool/intentcall/bin/release_train.dart sync, preserving the version argument for
the versioned target.
---
Nitpick comments:
In `@tool/intentcall/test/publish_preflight_test.dart`:
- Around line 339-394: The release-train fixture in _createReleaseTrainFixture
is only creating publishablePackages plus intentcall_gemma, but
syncReleaseTrainMetadata() operates over allInternalPackages. Update the fixture
setup to materialize every package from allInternalPackages (including
internal-only ones) so the test coverage stays aligned with the metadata sync
logic and won’t break when new internal packages are added.
- Around line 194-229: The release train sync test only covers dependency floor
and podspec updates, so it misses the publishable version rewrite path in
syncReleaseTrainMetadata(). Update the fixture setup in
publish_preflight_test.dart to make one publishable package pubspec.yaml stale
at 0.5.0, then extend the assertions after runReleaseTrainSync(repo) to verify
that the version is rewritten to 0.6.0, using the existing
release_train.runReleaseTrainSync and runReleaseTrainCheck flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a008adf-83ac-4192-b885-efcbfc7b6da4
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/release-pr-sync-train.ymljustfiletool/intentcall/bin/intentcall.darttool/intentcall/bin/release_train.darttool/intentcall/test/publish_preflight_test.dart
| permissions: | ||
| contents: write | ||
| pull-requests: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Drop the unused pull-requests: write permission.
This job only needs to push commits back to the branch. Keeping PR-write scope here unnecessarily broadens the token if the workflow is ever repurposed or a later step is added.
Suggested fix
permissions:
contents: write
- pull-requests: write📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: write | |
| pull-requests: write | |
| permissions: | |
| contents: write |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-pr-sync-train.yml around lines 12 - 14, The
workflow permissions are broader than needed because the release PR sync job
only pushes commits back to the branch, so the `pull-requests: write` scope
should be removed. Update the permissions block in the release sync workflow to
keep only the `contents: write` access, and leave the rest of the job unchanged.
| sync-release-train: | ||
| dart run tool/intentcall/bin/intentcall.dart sync-release-train | ||
|
|
||
| # Synchronize release train metadata to a specific version | ||
| sync-release-train-version version: | ||
| dart run tool/intentcall/bin/intentcall.dart sync-release-train --version {{version}} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## justfile targets\n'
git ls-files justfile
echo
echo '## relevant justfile excerpt'
nl -ba justfile | sed -n '45,70p'
echo
echo '## tool/intentcall/bin files'
git ls-files 'tool/intentcall/bin/*'
echo
echo '## search for release_train and sync-release-train references'
rg -n "release_train|sync-release-train|intentcall" justfile tool/intentcall/bin -g '!**/node_modules/**'
echo
echo '## outline release_train.dart if present'
if [ -f tool/intentcall/bin/release_train.dart ]; then
ast-grep outline tool/intentcall/bin/release_train.dart --view expanded || true
fiRepository: Arenukvern/intentcall
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## justfile excerpt'
sed -n '45,70p' justfile
echo
echo '## intentcall bin files'
git ls-files 'tool/intentcall/bin/*'
echo
echo '## release_train and sync references'
rg -n "release_train|sync-release-train|intentcall" justfile tool/intentcall/bin
echo
echo '## if release_train.dart exists, show its first 220 lines'
if [ -f tool/intentcall/bin/release_train.dart ]; then
sed -n '1,220p' tool/intentcall/bin/release_train.dart
fiRepository: Arenukvern/intentcall
Length of output: 15712
Keep the sync targets dependency-free. sync-release-train and sync-release-train-version still go through dart run, so they can fail in the same stale pub-resolution state they're meant to fix. Point them at dart tool/intentcall/bin/release_train.dart sync instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@justfile` around lines 53 - 58, The sync-release-train targets still invoke
the intentcall entrypoint through dart run, which can depend on stale pub
resolution; update both sync-release-train and sync-release-train-version in the
justfile to call the dedicated release_train.dart sync script directly with dart
tool/intentcall/bin/release_train.dart sync, preserving the version argument for
the versioned target.
Summary
dart pub getin CI so stale floors fail with a useful message instead of a solver error.just/CLI commands for maintainers.Validation
dart tool/intentcall/bin/release_train.dart checkdart run tool/intentcall/bin/intentcall.dart sync-release-train --checkdart test tool/intentcall/test/publish_preflight_test.dartdart format --output=none --set-exit-if-changed .just analyzejust testjust docs-checkdart run tool/intentcall/bin/intentcall.dart validatesteward probe --json --profile quicksteward benchmark --scenario intentcall.adapter-contract --jsonjust publish-dry-runSummary by CodeRabbit
New Features
Bug Fixes