Skip to content

Commit d6079ca

Browse files
Add initial PR review skills and skill validation hooks. (#9930)
* Add initial PR review skills. * Add common review patterns and remove duplication * fix relative paths * Add skill validation and clean up hooks.
1 parent d9b12be commit d6079ca

6 files changed

Lines changed: 175 additions & 1 deletion

File tree

.agents/hooks.json

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"skill-linter-and-review-hook": {
3+
"PostToolUse": [
4+
{
5+
"matcher": "write_to_file|replace_file_content|multi_replace_file_content",
6+
"hooks": [
7+
{
8+
"type": "command",
9+
"command": "./.agents/scripts/validate_skills_hook.sh",
10+
"timeout": 30
11+
}
12+
]
13+
}
14+
],
15+
"PostInvocation": [
16+
{
17+
"type": "command",
18+
"command": "./.agents/scripts/skill_post_invocation_hook.sh",
19+
"timeout": 30
20+
}
21+
]
22+
}
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/bin/bash
2+
3+
# Copyright 2026 The Flutter Authors
4+
# Use of this source code is governed by a BSD-style license that can be
5+
# found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
6+
7+
# Check if any skill file was modified in git working directory
8+
MODIFIED_SKILLS=$(git diff --name-only HEAD 2>/dev/null | grep "\.agents/skills/.*SKILL\.md$")
9+
10+
if [ -n "$MODIFIED_SKILLS" ]; then
11+
# Inject ephemeral message instructing AGY to review the modified skill
12+
cat <<EOF
13+
{
14+
"injectSteps": [
15+
{
16+
"ephemeralMessage": "AGY Skill Hook: The following skill files were modified: ${MODIFIED_SKILLS}. Please review the edited skills to ensure they follow best practices (conciseness, clear frontmatter) and verify that instructions are not repetitive or duplicative with existing style guides or skills."
17+
}
18+
]
19+
}
20+
EOF
21+
else
22+
echo "{}"
23+
fi
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/bin/bash
2+
3+
# Copyright 2026 The Flutter Authors
4+
# Use of this source code is governed by a BSD-style license that can be
5+
# found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
6+
7+
# Read AGY stdin JSON payload
8+
INPUT=$(cat)
9+
10+
# Extract TargetFile from tool call arguments
11+
TARGET_FILE=$(echo "$INPUT" | python3 -c "import sys, json; data=json.load(sys.stdin); print(data.get('toolCall', {}).get('args', {}).get('TargetFile', ''))" 2>/dev/null)
12+
13+
# Check if a skill file was modified
14+
if [[ "$TARGET_FILE" == *"SKILL.md"* ]] || [[ "$TARGET_FILE" == *".agents/skills"* ]]; then
15+
echo "Skill edit detected: $TARGET_FILE" >&2
16+
echo "Running dart_skills_lint..." >&2
17+
18+
# Run the skills linter from tool/ directory
19+
(cd tool && dart run dart_skills_lint:cli)
20+
fi
21+
22+
# PostToolUse expects an empty JSON object on stdout
23+
echo "{}"
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
name: reviewing-devtools-prs
3+
description: DevTools repository-specific PR review workflow enforcing DevTools style guidelines and common review patterns. Use when reviewing pull requests in the flutter/devtools repository.
4+
---
5+
6+
# Reviewing DevTools Pull Requests
7+
8+
Extends [reviewing-prs](../reviewing-prs/SKILL.md) for pull requests in `flutter/devtools`. Follow [reviewing-prs](../reviewing-prs/SKILL.md) for GitHub CLI data retrieval and the strict user approval workflow.
9+
10+
## References & Style Guidelines
11+
12+
Read and enforce:
13+
- **Style Guide**: [styleguide.md](../../../.gemini/styleguide.md) (severity tags `[MUST-FIX]`, `[CONCERN]`, `[NIT]`, zero-formatting policy, copyright headers, DRY rules, magic values)
14+
- **Repository Constraints**: [AGENTS.md](../../../AGENTS.md)
15+
- **Code Style**: [STYLE.md](../../../STYLE.md)
16+
17+
## Common Review Patterns
18+
19+
1. **Listener & Resource Disposals**:
20+
- Ensure controller and notifier listeners use `addAutoDisposeListener(...)`.
21+
22+
2. **Helper Widgets over Helper Methods**:
23+
- Prefer small composable `Widget` classes over helper methods returning `Widget` (`_buildFoo()`).
24+
25+
3. **Reuse Shared Components & Test Helpers**:
26+
- Use standard shared widgets (e.g. `CenteredMessage`) and test mocks (e.g. `mockConnectedApp`) instead of re-creating them inline.
27+
28+
4. **TODO Formatting**:
29+
- Link TODOs to a GitHub issue or LDAP: `// TODO(https://github.com/flutter/devtools/issues/<issue_number>): <description>`.
30+
31+
5. **Async & Unawaited Futures**:
32+
- Audit unawaited futures and suggest `unawaited(...)` or `safeUnawaited(...)` where appropriate.
33+
34+
6. **Feature Flags**:
35+
- Default feature flags to `false` with explicit test expectations in `feature_flags_test.dart`.
36+
37+
7. **Test File Structure & PR Scope**:
38+
- Place test fakes/helpers below `main()`.
39+
- Ask authors to revert unrelated file changes or commented-out test code.
40+
41+
8. **Constant Scoping**:
42+
- Keep single-use constants local to the component, but extract user-facing UI strings into shared constants when used across multiple places.
43+
44+
9. **Release Notes Scope (`NEXT_RELEASE_NOTES.md`)**:
45+
- Release notes are strictly for end-user facing changes (e.g. Inspector, Memory UI/UX). Internal tools (`dt` / `devtools_tool`), CI, and refactors are NOT user-facing.
46+
- Request removing release notes added for developer tools like `dt`, or suggest a `* <Description>. [#<PR_NUMBER>](https://github.com/flutter/devtools/pull/<PR_NUMBER>)` entry via [adding-release-notes](../adding-release-notes/SKILL.md) if a user-facing PR lacks one.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
name: reviewing-prs
3+
description: General workflow for fetching, inspecting, reviewing GitHub Pull Requests using the gh CLI, drafting user-aligned review comments, and securing approval before posting. Use when asked to review a GitHub PR or pull request.
4+
---
5+
6+
# Reviewing Pull Requests
7+
8+
This skill outlines the workflow for inspecting GitHub Pull Requests using the `gh` CLI, drafting review feedback, and securing user approval before posting review comments.
9+
10+
## Approval Safeguard (Strict Requirement)
11+
12+
> [!IMPORTANT]
13+
> **NEVER post comments or reviews to GitHub without explicit prior user approval.**
14+
> Always present draft review comments to the user in natural language first. Only execute write commands (`gh pr comment`, `gh pr review`) after the user approves.
15+
16+
## Workflow
17+
18+
### 1. Request Information via GitHub CLI
19+
20+
- **PR Details**:
21+
```bash
22+
gh pr view <pr-number> --repo <owner/repo> --json title,body,author,state,headRefName,baseRefName,comments,reviews,files
23+
```
24+
- **Code Diff**:
25+
```bash
26+
gh pr diff <pr-number> --repo <owner/repo>
27+
```
28+
- **Existing Inline Comments**:
29+
```bash
30+
gh api repos/<owner/repo>/pulls/<pr-number>/comments
31+
```
32+
- **CI / Status Checks**:
33+
```bash
34+
gh pr checks <pr-number> --repo <owner/repo>
35+
```
36+
37+
### 2. Inspect Context & Prior Feedback
38+
39+
- Read the PR description, linked issues, and full diff.
40+
- Verify whether existing bot or human comments have already been addressed in subsequent commits.
41+
42+
### 3. Draft Review Comments
43+
44+
- Keep comments direct, concise, and focused on code quality and correctness.
45+
- **Approvals**: Keep comments concise (`LGTM` or `A couple comments but lgtm.`). Avoid fluffy praise or re-summarizing the PR.
46+
- **Actionable Feedback**: Reference specific files, line numbers, and rationale when leaving suggestions.
47+
48+
### 4. Present Draft & Post Only Upon User Approval
49+
50+
> [!IMPORTANT]
51+
> **NEVER post comments or reviews to GitHub without explicit prior user approval.**
52+
53+
1. Present the drafted review comments to the user in your response window.
54+
2. Ask for confirmation: *"Would you like me to submit this review to GitHub?"*
55+
3. Once explicitly approved by the user, post the review:
56+
```bash
57+
gh pr review <pr-number> --repo <owner/repo> --comment --body "<approved review text>"
58+
```

.gemini/styleguide.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ and the CI testing ensures that the code is formatted correctly.
2727
* **Readability**: Code should be easy to understand for all contributors.
2828
* **Maintainability**: Code should be easy to modify and extend without breaking other screens.
2929
* **Consistency**: Adhering to consistent style across all DevTools packages improves collaboration and reduces errors.
30-
* **Code Reuse**: Use shared primitives and components rather than recreating them from scratch.
30+
* **Code Reuse & DRY**: Use shared primitives and components rather than recreating them from scratch. Flag duplicated code blocks (90%+ identical) and repeated sub-expressions or function calls within the same method as `[CONCERN]` or `[NIT]`, and suggest extracting them into local variables or constants.
31+
* **Constants & Magic Values**: Avoid using raw/magic strings or numbers. Extract them into descriptive named constants so the intent and meaning of the values are clear.
3132
* **Testing**: All changes should include automated tests to ensure correctness and prevent regressions.
3233

3334
## 3. Guidelines from Existing Documentation

0 commit comments

Comments
 (0)