Skip to content

Commit 524c52e

Browse files
authored
Make code review agent more robust (open-telemetry#17249)
1 parent 6bdd5af commit 524c52e

1 file changed

Lines changed: 40 additions & 19 deletions

File tree

.github/agents/code-review-and-fix.agent.md

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ Primary responsibilities:
1919

2020
Do not stop until all in-scope files are reviewed and fixed where possible.
2121

22+
## Knowledge Loading
23+
24+
Always load:
25+
26+
- `docs/contributing/style-guide.md`
27+
- `knowledge/general-rules.md` — review checklist and core rules
28+
29+
Load other knowledge files only when their scope trigger applies.
30+
Use the **Knowledge File** column in the checklist table.
31+
32+
## Review Checklist and Core Rules
33+
34+
Load `knowledge/general-rules.md` — it contains the review checklist table and all
35+
core rules that apply to every review.
36+
2237
## Scope Modes
2338

2439
Determine scope from the user request:
@@ -266,7 +281,7 @@ Output content rules:
266281
- When writing structured output to a file, write only the requested payload. Do not wrap it in Markdown fences,
267282
add headings, or include extra commentary before or after it.
268283

269-
### Phase 4: Validate and Report
284+
### Phase 4: Validate
270285

271286
**All Gradle commands in this phase must use timeout `0` (no timeout). In this repository,
272287
legitimate Gradle validation runs can take 10 minutes or more. Never set a finite timeout,
@@ -285,6 +300,11 @@ until the previous one has definitively completed and you have observed its fina
285300
status. If a prior run may still be active, first wait for it or confirm its completion
286301
before proceeding.
287302

303+
Do not move on to Phase 5 until every Gradle command started in Phase 4 has either:
304+
305+
1. completed and you have observed its final exit status, or
306+
2. been explicitly recorded as a validation limitation after the recovery loop below.
307+
288308
If a command-execution attempt fails for tool-related reasons, follow this recovery loop before
289309
reporting a limitation:
290310

@@ -349,6 +369,10 @@ Execute these steps strictly in order — do not reorder:
349369
`:instrumentation:foo:foo-1.0:library`,
350370
`:instrumentation:foo:foo-1.0:javaagent`, and any version-variant siblings such as
351371
`:instrumentation:foo:foo-2.0:library` if it depends on the `foo-1.0:testing` module.
372+
373+
Do not move on to step 2 until every required `:check` run from this step, including
374+
sibling-module validation and any re-runs after fixes or reverts, has fully completed
375+
and you have observed the final exit status for each run.
352376
2. **Run muzzle validation when muzzle config changed.** If any review fix touched Gradle
353377
muzzle configuration (for example `muzzle {}`, version ranges, `assertInverse.set(true)`,
354378
or module wiring affecting muzzle), run the relevant module's `:muzzle` task:
@@ -371,17 +395,29 @@ Execute these steps strictly in order — do not reorder:
371395
`Needs Manual Fix` in the final output with a note explaining the muzzle failure.
372396
4. After reverting, re-run the `:muzzle` task to confirm the revert restored a green
373397
build. Never commit code that fails muzzle validation.
398+
399+
Do not move on to step 3 until every required `:muzzle` run from this step, including
400+
any re-runs after fixes or reverts, has fully completed and you have observed the final
401+
exit status for each run.
374402
3. **Last, after all validation is done**, run `./gradlew spotlessApply` to fix formatting
375403
across all modified files.
376404
`spotlessApply` must be the final build command — never run it before tests or muzzle.
377405
Before running it, confirm that no earlier Gradle validation command is still running.
378-
4. **Verify substantive changes remain.** Run `git diff --ignore-all-space --ignore-blank-lines`
406+
407+
Do not move on to Phase 5 until `spotlessApply` has fully completed and you have
408+
observed its final exit status.
409+
410+
### Phase 5: Finalize and Report
411+
412+
Do not begin Phase 5 until Phase 4 is fully closed out.
413+
414+
1. **Verify substantive changes remain.** Run `git diff --ignore-all-space --ignore-blank-lines`
379415
and confirm non-empty output. If the only remaining diffs are whitespace changes — or if
380416
all review fixes were reverted during validation — **stop here**: reset the working tree
381417
(`git checkout -- .`), do not commit or push. If any reverted items were recorded as
382418
`Needs Manual Fix`, emit the final output with those items. Otherwise report
383419
"No issues found." and exit.
384-
5. Commit all changes in a single commit. The subject line must always be
420+
2. Commit all changes in a single commit. The subject line must always be
385421
`Review fixes for <module>` where `<module>` is the short module name (e.g.,
386422
`apache-elasticjob-3.0 javaagent`). The body is a bulleted list of changes:
387423

@@ -399,24 +435,9 @@ Execute these steps strictly in order — do not reorder:
399435
```
400436

401437
Create exactly one commit for all fixes — do not commit incrementally.
402-
6. Produce the final output in the format requested by the caller.
438+
3. Produce the final output in the format requested by the caller.
403439

404440
The caller must define the final output format or schema. Follow that request exactly:
405441

406442
- Do **not** add headings, commentary, or fallback prose unless the caller asks for them.
407443
- Preserve the recorded per-change reasons in whatever output format the caller requested.
408-
409-
## Knowledge Loading
410-
411-
Always load:
412-
413-
- `docs/contributing/style-guide.md`
414-
- `knowledge/general-rules.md` — review checklist and core rules
415-
416-
Load other knowledge files only when their scope trigger applies.
417-
Use the **Knowledge File** column in the checklist table.
418-
419-
## Review Checklist and Core Rules
420-
421-
Load `knowledge/general-rules.md` — it contains the review checklist table and all
422-
core rules that apply to every review.

0 commit comments

Comments
 (0)