requirement_nudger: Initial commit
Change-Id: I986af19cc0feaeb345b8294f142d0b85f666a781
Bug: b/416550827
Reviewed-on: https://pigweed-review.googlesource.com/c/infra/recipes/+/299252
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
Reviewed-by: Oliver Newman <olivernewman@google.com>
Commit-Queue: Rob Mohr <mohrr@google.com>
Pigweed-Auto-Submit: Rob Mohr <mohrr@google.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/recipes/requirement_nudger.proto b/recipes/requirement_nudger.proto
new file mode 100644
index 0000000..09e3e3c
--- /dev/null
+++ b/recipes/requirement_nudger.proto
@@ -0,0 +1,45 @@
+// Copyright 2025 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+syntax = "proto3";
+
+import "recipe_modules/pigweed/checkout/options.proto";
+
+package recipes.pigweed.requirement_nudger;
+
+message HostConfig {
+ // Label to trigger auto-submission.
+ string auto_submit_label = 1;
+
+ // Gerrit host to run against.
+ string gerrit_host = 2;
+}
+
+message InputProperties {
+ repeated HostConfig host_configs = 1;
+
+ // Number of days to store history about commenting on changes. After this
+ // it's possible a change will be commented on again.
+ int32 history_retention_days = 2;
+
+ // Maximum amount of time without activity to comment. Default: 2. Mostly this
+ // is to prevent resurrecting old changes when this is turned on.
+ int32 max_age_days = 3;
+
+ // Minimum amount of time without activity before commenting. Default: 30.
+ int32 min_age_minutes = 4;
+
+ // Whether to actually comment on changes.
+ bool dry_run = 5;
+}
diff --git a/recipes/requirement_nudger.py b/recipes/requirement_nudger.py
new file mode 100644
index 0000000..def2172
--- /dev/null
+++ b/recipes/requirement_nudger.py
@@ -0,0 +1,486 @@
+# Copyright 2025 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+"""Comment on changes that are missing "trivial" submit requirements."""
+
+from __future__ import annotations
+
+from collections.abc import Mapping
+import dataclasses
+import datetime
+import json
+import re
+from typing import Any, Generator, TypedDict
+
+from google.protobuf import json_format
+from PB.go.chromium.org.luci.buildbucket.proto import (
+ build as build_pb,
+ common as common_pb,
+)
+from PB.recipes.pigweed.requirement_nudger import (
+ HostConfig,
+ InputProperties,
+)
+from PB.recipe_engine import result as result_pb
+from recipe_engine import (
+ config_types,
+ engine_types,
+ post_process,
+ recipe_api,
+ recipe_test_api,
+)
+
+DEPS = [
+ 'fuchsia/buildbucket_util',
+ 'fuchsia/builder_state',
+ 'fuchsia/gerrit',
+ 'recipe_engine/buildbucket',
+ 'recipe_engine/json',
+ 'recipe_engine/properties',
+ 'recipe_engine/step',
+ 'recipe_engine/time',
+]
+
+PROPERTIES = InputProperties
+
+_GOOD_REQUIREMENT_STATUSES = frozenset(
+ 'SATISFIED OVERRIDDEN NOT_APPLICABLE FORCED'.split()
+)
+_BAD_REQUIREMENT_STATUSES = frozenset('UNSATISFIED ERROR'.split())
+_REQUIREMENT_STATUSES = _GOOD_REQUIREMENT_STATUSES.union(
+ _BAD_REQUIREMENT_STATUSES
+)
+
+
+def _find_changes(
+ api: recipe_api.RecipeScriptApi,
+ gerrit_host: str,
+ auto_submit_label: str,
+ max_age_days: int,
+ min_age_minutes: int,
+):
+ assert gerrit_host
+ assert auto_submit_label
+
+ search_parts: list[str] = []
+
+ # Ignore WIP changes. People don't expect them to be ready to submit.
+ search_parts.append('-is:wip')
+
+ # Ignore changes without CR+2.
+ search_parts.append('label:Code-Review=MAX')
+
+ # Only process changes with auto-submit set. People only expect these
+ # changes to end up submitted without further interaction.
+ search_parts.append(f'label:{auto_submit_label}=MAX')
+
+ # Ignore submitted/abandoned changes.
+ search_parts.append('status:NEW')
+
+ # Ignore changes with all requirements met. We definitely don't need to
+ # comment on these changes.
+ search_parts.append('-is:submittable')
+
+ # Ignore changes with unresolved comments. The unresolved comments
+ # should be sufficiently visible to change owners.
+ search_parts.append('-has:unresolved')
+
+ # Only look at stuff from the past couple days.
+ search_parts.append(f'-age:{max_age_days}d')
+
+ # Wait until a change has been left alone for a short period.
+ search_parts.append(f'age:{min_age_minutes}m')
+
+ return api.gerrit.change_query(
+ 'find changes',
+ ' '.join(search_parts),
+ host=gerrit_host,
+ max_attempts=2,
+ timeout=30,
+ ).json.output
+
+
+def _process_change(
+ api: recipe_api.RecipeScriptApi,
+ pres: engine_types.StepPresentation,
+ dry_run: bool,
+ now: int,
+ state: dict[str, dict[str, int]],
+ host: str,
+ change_id: int,
+) -> int:
+ if host in state and change_id in state[host]:
+ pres.step_summary_text = 'Already commented on this change'
+ api.step.empty('already commented')
+ return 0
+
+ bb_input = api.buildbucket.build.input
+ details: dict[str, Any] = api.gerrit.change_details(
+ 'change details',
+ change_id=str(change_id),
+ host=host,
+ query_params=['SUBMIT_REQUIREMENTS'],
+ ).json.output
+
+ submit_requirements = {
+ x['name']: x for x in details.get('submit_requirements', [])
+ }
+
+ unmet: set[str] = set()
+ for name, req in submit_requirements.items():
+ if req['status'] in _BAD_REQUIREMENT_STATUSES:
+ unmet.add(name)
+ elif req['status'] not in _GOOD_REQUIREMENT_STATUSES:
+ assert False, f'bad status {req["status"]}' # pragma: no cover
+
+ if not unmet:
+ # This shouldn't really happen, because we should only be getting
+ # changes that are unsubmittable. However, the change could have been
+ # modified between searching for changes and getting change details.
+ pres.step_summary_text = 'All requirements met'
+
+ unhandled: set[str] = set()
+ for name in unmet:
+ if (
+ name not in ('Docs', 'Images-Not-Allowed')
+ and not name.endswith('-Tests')
+ and not name.endswith('-Readability')
+ ):
+ unhandled.add(name)
+
+ if unhandled:
+ pres.step_summary_text = (
+ 'Unhandled unsatisfied submit '
+ f'requirement{"" if len(unhandled) == 1 else "s"}: '
+ f'{", ".join(sorted(unhandled))}'
+ )
+ # Clear unmet instead of returning so we get the "commented about" step.
+ unmet = set()
+
+ def comment(name, message):
+ if dry_run:
+ pres = api.step.empty(name).presentation
+ pres.step_summary_text = message
+
+ else:
+ api.gerrit.set_review(
+ name,
+ change_id,
+ host=host,
+ message=message,
+ revision=details['current_revision'],
+ notify='OWNER_REVIEWERS',
+ )
+
+ comments = []
+ for name in unmet:
+ if name == 'Docs':
+ comment(
+ 'Docs',
+ 'The "Docs" submit requirement is not met. If there are no '
+ 'updates to documentation in this commit, add them. '
+ 'Alternatively, vote +1 on the Docs-Not-Needed label. If '
+ 'Docs-Not-Needed already has a +1 vote, then there are docs '
+ 'updates in this change and the change owner has voted +1. '
+ 'Remove the +1 vote and the change will become submittable.',
+ )
+ comments.append(name)
+
+ elif name == 'Images-Not-Allowed':
+ comment(
+ 'Images-Not-Allowed',
+ 'Images are not allowed in the Pigweed repository. If you '
+ 'believe this is an exception, get the current Pigweed oncall '
+ 'user to vote +1 on Images-Allowed. See the [Pigweed console]'
+ '(https://ci.chromium.org/p/pigweed/g/pigweed.pigweed/console) '
+ 'to find out who is oncall. If you are oncall, get a Pigweed '
+ 'Gerrit admin to vote +1. They will need a bug to reference to '
+ 'gain this permission.',
+ )
+ comments.append(name)
+
+ elif name.endswith('-Tests'):
+ language = name.rsplit('-', 1)[0]
+ comment(
+ name,
+ f'Changes to {language} files also require updates to test '
+ 'files. Add changes to test files (with a "_test" suffix on '
+ "the name). If this change doesn't need tests, vote +1 on "
+ 'Tests-Not-Needed. If you already have a +1 vote on that label '
+ 'then you likely already have an update to a test, and you '
+ 'need to remove the vote.',
+ )
+ comments.append(name)
+
+ elif name.endswith('-Readability'):
+ language = name.rsplit('-', 1)[0]
+ comment(
+ name,
+ f'Changes to {language} files also require readability '
+ 'approval.',
+ )
+ comments.append(name)
+
+ if comments:
+ state.setdefault(host, {})
+ state[host][change_id] = now
+ api.builder_state.save(state)
+
+ api.step.empty(
+ f'commented about {len(comments)} '
+ f'requirement{"" if len(comments) == 1 else "s"}',
+ )
+
+ return len(comments)
+
+
+def _gerrit_link(host: str, number: int | str) -> str:
+ return (
+ f'[{host}:{number}](https://{host}-review.googlesource.com/c/{number})'
+ )
+
+
+def RunSteps(
+ api: recipe_api.RecipeScriptApi,
+ props: InputProperties,
+) -> result_pb.RawResult | None: # pragma: no cover
+ """Comment on changes that are missing "trivial" submit requirements."""
+ props.dry_run = props.dry_run or api.buildbucket_util.is_dev_or_try
+ props.history_retention_days = props.history_retention_days or 30
+ props.max_age_days = props.max_age_days or 2
+ props.min_age_minutes = props.min_age_minutes or 30
+
+ # We could just grab the time at different points throughout the recipe, but
+ # things are easier to test if we pretend the runtime is zero.
+ now = int(api.time.time())
+
+ with api.step.nest('state'):
+ old_state: dict[str, dict[str, int]] = (
+ api.builder_state.fetch_previous_state()
+ )
+
+ threshold = now - props.history_retention_days * 24 * 60 * 60
+ pres = api.step.empty('threshold').presentation
+ pres.step_summary_text = str(threshold)
+
+ # Only keep entries newer than the threshold.
+ state: dict[str, dict[str, int]] = {}
+ for host, entries in old_state.items():
+ for change_id, timestamp in entries.items():
+ if timestamp > threshold:
+ state.setdefault(host, {})
+ state[host][change_id] = timestamp
+
+ api.builder_state.save(state)
+
+ summary = []
+ for host_config in props.host_configs:
+ short_host = api.gerrit.host_basename(host_config.gerrit_host)
+ with api.step.nest(short_host):
+ changes = _find_changes(
+ api,
+ gerrit_host=short_host,
+ auto_submit_label=host_config.auto_submit_label,
+ max_age_days=props.max_age_days,
+ min_age_minutes=props.min_age_minutes,
+ )
+
+ for change in changes:
+ change_id = str(change['_number'])
+ with api.step.nest(change_id) as pres:
+ num_comments = _process_change(
+ api=api,
+ pres=pres,
+ dry_run=props.dry_run,
+ now=now,
+ state=state,
+ host=short_host,
+ change_id=change_id,
+ )
+
+ if num_comments:
+ summary.append(
+ f'* {_gerrit_link(short_host, change_id)}: '
+ f'{num_comments}'
+ )
+
+ return result_pb.RawResult(
+ summary_markdown='\n'.join(summary),
+ status=common_pb.SUCCESS,
+ )
+
+
+def GenTests(api) -> Generator[recipe_test_api.TestData, None, None]:
+ """Create tests."""
+
+ @dataclasses.dataclass
+ class Entry:
+ change_id: int
+ timestamp: int
+ host: str = 'pigweed'
+
+ def state(*entries):
+ res = {}
+ for entry in entries:
+ res.setdefault(entry.host, {})
+ res[entry.host][entry.change_id] = entry.timestamp
+ return api.builder_state(res, prefix='state')
+
+ def host_config(host: str = 'pigweed', label: str = 'Pigweed-Auto-Submit'):
+ return HostConfig(gerrit_host=host, auto_submit_label=label)
+
+ def props(*host_configs, **kwargs):
+ if not host_configs:
+ host_configs = (host_config(),)
+ res = InputProperties(**kwargs)
+ res.host_configs.extend(host_configs)
+ return api.properties(res)
+
+ def search_results(*change_number: int, host: str = 'pigweed'):
+ return api.step_data(
+ f'{host}.find changes',
+ api.json.output([{'_number': int(x)} for x in change_number]),
+ )
+
+ def change_details(
+ change_number: int,
+ met_requirements: Sequence[str] = (),
+ unmet_requirements: Sequence[str] = (),
+ host: str = 'pigweed',
+ ):
+ reqs = []
+ for i, req in enumerate(met_requirements):
+ status = 'SATISFIED OVERRIDDEN NOT_APPLICABLE'.split()[i % 3]
+ reqs.append({'name': req, 'status': status})
+
+ for req in unmet_requirements:
+ reqs.append({'name': req, 'status': 'UNSATISFIED'})
+
+ return api.step_data(
+ f'{host}.{change_number}.change details',
+ api.json.output(
+ {'current_revision': 'HASH', 'submit_requirements': reqs},
+ ),
+ )
+
+ def assert_already_commented(
+ change_number: int,
+ gerrit_host: str = 'pigweed',
+ ):
+ return api.post_process(
+ post_process.MustRun,
+ f'{gerrit_host}.{change_number}.already commented',
+ )
+
+ def assert_commented(
+ change_number: int,
+ requirement_name: str,
+ gerrit_host: str = 'pigweed',
+ ):
+ return api.post_process(
+ post_process.MustRun,
+ f'{gerrit_host}.{change_number}.{requirement_name}',
+ )
+
+ def assert_num_comments(
+ change_number: int,
+ num: int,
+ gerrit_host: str = 'pigweed',
+ ):
+ return api.post_process(
+ post_process.MustRun,
+ f'{gerrit_host}.{change_number}.commented about {num} '
+ f'requirement{"" if num == 1 else "s"}',
+ )
+
+ def drop_expectations_must_be_last():
+ # No need for expectation files, everything of note here is tested by
+ # assertions. This must be the last thing added to the test.
+ return api.post_process(post_process.DropExpectation)
+
+ yield api.test(
+ 'empty',
+ props(),
+ search_results(),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'none',
+ props(),
+ search_results(1234),
+ change_details(1234),
+ assert_num_comments(1234, 0),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'no-code-review',
+ props(),
+ search_results(1234),
+ change_details(
+ 1234,
+ unmet_requirements=['Code-Review', 'Docs'],
+ met_requirements=['Code-Owners', 'API-Review', 'Python-Tests'],
+ ),
+ assert_num_comments(1234, 0),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'docs-purgestate',
+ state(Entry(1234, 1000000000)),
+ props(),
+ search_results(1234),
+ change_details(1234, unmet_requirements=['Docs']),
+ assert_commented(1234, 'Docs'),
+ assert_num_comments(1234, 1),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'images-readability',
+ props(dry_run=True),
+ search_results(1234),
+ change_details(
+ 1234,
+ unmet_requirements=['Images-Not-Allowed', 'Batch-Readability'],
+ ),
+ assert_commented(1234, 'Images-Not-Allowed'),
+ assert_commented(1234, 'Batch-Readability'),
+ assert_num_comments(1234, 2),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'tests',
+ props(),
+ search_results(1234),
+ change_details(
+ 1234,
+ unmet_requirements=['Python-Tests', 'Batch-Tests'],
+ ),
+ assert_commented(1234, 'Python-Tests'),
+ assert_commented(1234, 'Batch-Tests'),
+ assert_num_comments(1234, 2),
+ drop_expectations_must_be_last(),
+ )
+
+ yield api.test(
+ 'docs-state',
+ state(Entry(1234, 1335000000)),
+ props(),
+ search_results(1234),
+ assert_already_commented(1234),
+ drop_expectations_must_be_last(),
+ )
