[camera_android_camerax] Correct `pre-push` skill version validation logic by camsim99 · Pull Request #12371 · flutter/packages

camsim99 · 2026-08-04T22:30:18Z

Corrects the version and merge conflict validation logic in the pre-push skill to compare the local branch against the upstream branch. If the merge conflict check fails, then the skill immediately reports that the code is not ready to push now.

Also, adds an eval to ensure this update works as expected. Result of running it:

Eval ID	Scenario	Expected Outcome	Result	Justification
1	Behind Upstream (`setup_behind_upstream.sh`)	Detect branch is behind `upstream/main`, stop immediately without updating release info	PASS	Checked merge-base against `upstream/main`, halted immediately with `# NO, you are not ready to push`, and left `pubspec.yaml` and `CHANGELOG.md` unmodified.
2	Behind Upstream with Conflicts (`setup_behind_upstream_conflict.dart`)	Detect that the branch is behind the upstream repository	PASS	Detected that the branch is behind the upstream repository, Ran git merge-tree and detected merge conflicts (CONFLICT (content): Merge conflict in .ci/flutter_master.version), Stopped immediately without running subsequent checks, Output # NO, you are not ready to push. and provided actionable instructions on how to resolve conflicts before pushing

Ideally #12369 lands first so I can update the references of origin to upstream.

Follow up from one-shot attempt #12302. See go/flutter-project-one-shot for more information on the project.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

flutter-dashboard · 2026-08-04T22:30:25Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request updates the pre-push skill documentation and evaluation scripts to use upstream/main instead of origin/main for branch comparisons, and introduces new evaluation test cases and setup scripts to verify the skill's behavior. The feedback suggests fetching with a depth of at least 2 in setup_behind_upstream.sh to prevent failures in shallow-clone environments, and updating comments in the setup scripts to consistently reference upstream/main instead of origin/main.

reidbaker · 2026-08-14T18:37:45Z

+    '$upstream/main~1..$upstream/main',
+    '--name-only',
+  ], workingDirectory: packageDir.path);
+  if (diffResult.exitCode != 0) {


There are probably corner cases here around empty commits or commits that modify other packages etc. I am not sure how much time we want to spend on those usecases before landing this pr.

Maybe have your agent brainstorm in specifically this areas and see if there are any non complex changes you can make to handle the more likely set of changes.

The agent decided to go back 10 commits rather than 2 for greater chances of finding a conflict and added a fallback for specifically filtering for a modified file by looking upstream and adds another change to a file to ensure a modified file is picked up to begin with. PTAL

reidbaker · 2026-08-14T18:38:46Z

+    exit(1);
+  }
+
+  final ProcessResult commitResult = await Process.run('git', <String>[


nit and not blocking for this pr. If this test data code folder grows we should probably extract some of these to shared code.

reidbaker · 2026-08-14T18:39:51Z

+      "expected_chat_output": [
+        "The agent detected that the branch is behind the upstream main branch.",
+        "The agent verified that there are no merge conflicts with the upstream main branch.",
+        "The agent warned the user that the branch must be pulled or merged before pushing.",


This line and line 16 are the only lines that actually have expections against chat output. The rest need to move to expected repo state.

…os_version

reidbaker · 2026-08-24T18:27:46Z

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: avoid_print


Ignore for file is something agents usually do and it normally an codesmell. Confirm you are ok with it and or there is not better alternative.

to the point of I am considering banning ignore_for_file in the codebase entirely it happens so often and is so likely to be wrong.

reidbaker · 2026-08-24T18:29:25Z

+    'remote',
+    '-v',
+  ], workingDirectory: workingDirectory);
+  if (result.exitCode != 0) {


Can you describe an error we would get from git remote -v where an error code means that we should return "upstream"?

reidbaker · 2026-08-24T18:30:32Z

+/// conflicts with the latest changes on upstream/main, and verifies that
+/// pre-push-skill detects the conflict and stops immediately.
+void main() async {
+  // The root of the packages directory.


nonblocking nit: Can you document what directory this is suppose to be?

reidbaker · 2026-08-24T18:31:27Z

+  // 2. Find a modified file in recent commits of upstream/main that exists in the repo
+  print('Finding modified file in recent upstream commits...');
+  String? fileToConflict;
+  for (var i = 1; i <= 5; i++) {


Can you add a comment for why we are looping 5 times?

reidbaker · 2026-08-24T18:32:19Z

+    final ProcessResult diffResult = await Process.run('git', <String>[
+      'diff',
+      '--name-only',
+      '--diff-filter=M',


can/should diff-filter include a filter for commits that have modified the package we are interested in?

reidbaker · 2026-08-24T18:33:18Z

+      '--diff-filter=M',
+      '$upstream/main~$i..$upstream/main',
+    ], workingDirectory: repoRoot.path);
+    if (diffResult.exitCode == 0) {


instead of doing this logic inside the if on the good case can you check for the bad case, return out early and then let this code be after the bad case?

reidbaker · 2026-08-24T18:36:21Z

+import 'dart:io';
+
+/// Detects the remote name pointing to the main flutter/packages repository.
+Future<String> getUpstreamRemote(String workingDirectory) async {


I am open to the argument that it is out of scope especially because you are just trying to write good evals for your good skill updates. BUT, this appears to be duplicate code and not super simple duplicate code. Lets figure out how to share code or maybe land your skill changes and keep working on the evals.

reidbaker · 2026-08-24T18:39:04Z

 ```bash
-git fetch origin main
-git merge-base --is-ancestor origin/main HEAD
+UPSTREAM_PREPUSH=$(git remote -v | grep 'flutter/packages' | head -n 1 | awk '{print $1}')


Nonblocking. These are fine for now. I think there is a level of complexity where we would want the bash commands to instead run scripts from the skills directory. One of the advantages is we could have the evals "mock" the output of the script and throw a non zero exit code then behave as if there is a conflict or we could verify that the script was actually run. Eventually if this part is slow we might even build in caching to the script if we wanted.

auto-submit · 2026-09-10T20:30:20Z

autosubmit label was removed for flutter/packages/12371, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

…os_version

…er#193641) flutter/packages@0ba9a82...d5ec6db 2026-10-01 faheemabbas766@gmail.com [tool] Enforce README package table order (flutter/packages#12316) 2026-09-30 jessiewong401@gmail.com [various] Allow plugin example apps to build and test on JDK 25 (flutter/packages#13031) 2026-09-30 149176071+m1roxx@users.noreply.github.com [go_router] Expose Navigator clipBehavior on ShellRoute and StatefulShellBranch (flutter/packages#12646) 2026-09-30 stuartmorgan@google.com [google_maps_flutter] Convert unit tests to Kotlin (flutter/packages#13072) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 ListTile template to use new gen_defaults (flutter/packages#13056) 2026-09-30 15619084+vashworth@users.noreply.github.com Allow tests to use macOS 15.7 or macOS 26.6 (flutter/packages#13007) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix exposure offset setting error thrown when canceled by a new request (flutter/packages#12582) 2026-09-30 engine-flutter-autoroll@skia.org Roll Flutter from 55b8f88 to d649d2b (27 revisions) (flutter/packages#13080) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 InputDecorator template to use new gen_defaults (flutter/packages#13024) 2026-09-30 tarrinneal@gmail.com [pigeon] Fix JNI/FFI typed data memory lifetime bugs and update docs (flutter/packages#13061) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Correct `pre-push` skill version validation logic (flutter/packages#12371) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

camsim99 added 2 commits August 4, 2026 15:03

attempt solving versioning issue

62d9ade

add eval for changes

418ae5b

flutter-dashboard Bot added the CICD Run CI/CD label Aug 4, 2026

camsim99 added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Aug 4, 2026

github-actions Bot added the p: camera label Aug 4, 2026

gemini-code-assist Bot reviewed Aug 4, 2026

View reviewed changes

camsim99 mentioned this pull request Aug 4, 2026

[camera_android_camerax] Restore torch state when camera changes #12302

Closed

7 of 11 tasks

camsim99 added 3 commits August 5, 2026 10:32

Merge remote-tracking branch 'upstream/main' into cos_version

087d37a

clean up PR

9893090

address review

7d417fa

camsim99 requested a review from reidbaker August 5, 2026 17:40

reidbaker reviewed Aug 6, 2026

View reviewed changes

Comment thread packages/camera/camera_android_camerax/.agents/skills/pre-push-skill/SKILL.md

camsim99 added 3 commits August 13, 2026 10:03

address review

bd85b8a

analyze, format

085d42f

add root comment

07049ff

camsim99 requested a review from reidbaker August 13, 2026 19:37

format code

d25423b

reidbaker reviewed Aug 14, 2026

View reviewed changes

camsim99 added 3 commits August 19, 2026 10:24

address review

5e3648b

Merge remote-tracking branch 'refs/remotes/origin/cos_version' into c…

1199c8d

…os_version

nit

80d7115

camsim99 requested a review from reidbaker August 19, 2026 17:33

reidbaker requested changes Aug 19, 2026

View reviewed changes

Comment thread packages/camera/camera_android_camerax/.agents/skills/pre-push-skill/evals/evals.json

Comment thread packages/camera/camera_android_camerax/.agents/skills/pre-push-skill/evals/evals.json Outdated

camsim99 added 2 commits August 20, 2026 09:23

remove logic conflicts

c2aa27a

get rid of e.g.

e17edd3

camsim99 requested a review from reidbaker August 20, 2026 16:33

Merge remote-tracking branch 'upstream/main' into cos_version

2044719

Fix bad merge

b4bac21

reidbaker reviewed Aug 24, 2026

View reviewed changes

camsim99 requested a review from reidbaker September 2, 2026 19:25

reidbaker added 3 commits September 10, 2026 16:16

Merge branch 'main' into cos_version

08971d9

Fix formatting in evals.json for expected_repo_state

2caf60c

Fix formatting in evals.json for expected_repo_state

7b939be

reidbaker approved these changes Sep 10, 2026

View reviewed changes

reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 10, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 10, 2026

camsim99 added 3 commits September 29, 2026 16:38

Merge remote-tracking branch 'upstream/main' into cos_version

595fb1b

fix error

65530d0

Merge remote-tracking branch 'refs/remotes/origin/cos_version' into c…

b6e16a0

…os_version

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 30, 2026

auto-submit Bot merged commit d4fb76a into flutter:main Sep 30, 2026
13 checks passed

engine-flutter-autoroll mentioned this pull request Oct 1, 2026

Roll Packages from 0ba9a8241e40 to d5ec6dbaf1c6 (11 revisions) flutter/flutter#193641

Merged

Conversation

camsim99 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

flutter-dashboard Bot commented Aug 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

auto-submit Bot commented Sep 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

camsim99 commented Aug 4, 2026 •

edited

Loading