Add Agent Playbook section for automated updates - #81
Conversation
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The pull request successfully introduces a detailed 'Agent Playbook' for repository updates; however, it introduces a major documentation conflict that should be resolved before merging.
Specifically, the new playbook correctly identifies documentation generation as a mandatory step, while the existing README text at line 29 explicitly labels it as 'obsolete'. This contradiction is likely to cause failures or skipped steps when the playbook is executed by AI agents. Additionally, minor optimizations for local build commands and git push practices are suggested to improve the developer/agent experience.
About this PR
- Providing contradictory instructions within the same document undermines the purpose of an 'Agent Playbook'. Automated agents may behave unpredictably when faced with a 'mandatory' instruction that is elsewhere described as 'obsolete'. Ensure all legacy update instructions (lines 25-31) are fully decommissioned.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| We use the [codacy-plugins-test](https://github.com/codacy/codacy-plugins-test) to test our external tools integration. | ||
| You can follow the instructions there to make sure your tool is working as expected. | ||
|
|
||
| ## Agent Playbook: Updating This Repository End-to-End |
There was a problem hiding this comment.
🔴 HIGH RISK
This playbook introduces a comprehensive guide that overlaps with and contradicts the existing 'Usage' (lines 10-23) and 'Bump bandit version' (lines 25-31) sections. Specifically, line 29 incorrectly states that documentation generation is 'obsolete', whereas this new section identifies it as a mandatory requirement. To prevent AI agents from encountering conflicting instructions, remove or consolidate the older sections.
Recommended action: Remove lines 25-31 to centralize instructions in the new Playbook section.
| 6. **Iterate on failures**, re-running only the relevant DockerTest command after each fix. | ||
| 7. **Commit** the version bump together with the regenerated `docs/` files in one change. | ||
| 8. **Push and open a PR.** CI runs `checkout_and_version` -> `publish_docker_local` -> `plugins_test` -> `publish_docker` (master only) -> `tag_version`. | ||
| 9. **Poll the PR's real CI checks until they all pass — local validation is NOT the finish line.** After every push, run `gh pr checks <pr-url>` and keep re-polling (short sleep while any check is `pending`) until all checks finish. If a check fails, fetch its actual log (CircleCI API/UI for the failing job — don't guess), find the true root cause, fix it, push again (never `--no-verify`, never force-push), and re-poll. Repeat until every check is green. The CI environment's toolchain can differ from your local one, so a clean local run does not guarantee CI passes. Only stop iterating when every check passes, or you hit a genuine product/infra decision that needs a human — in which case explain it in the PR rather than guessing. |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Prohibiting force-pushes entirely can block an agent from rebasing or cleaning up its own commit history during the PR iteration process. Using --force-with-lease is the standard safe practice for PR branches.
| 9. **Poll the PR's real CI checks until they all pass — local validation is NOT the finish line.** After every push, run `gh pr checks <pr-url>` and keep re-polling (short sleep while any check is `pending`) until all checks finish. If a check fails, fetch its actual log (CircleCI API/UI for the failing job — don't guess), find the true root cause, fix it, push again (never `--no-verify`, never force-push), and re-poll. Repeat until every check is green. The CI environment's toolchain can differ from your local one, so a clean local run does not guarantee CI passes. Only stop iterating when every check passes, or you hit a genuine product/infra decision that needs a human — in which case explain it in the PR rather than guessing. | |
| 9. **Poll the PR's real CI checks until they all pass — local validation is NOT the finish line.** After every push, run `gh pr checks <pr-url>` and keep re-polling (short sleep while any check is `pending`) until all checks finish. If a check fails, fetch its actual log (CircleCI API/UI for the failing job — don't guess), find the true root cause, fix it, push again (never `--no-verify`, prefer `--force-with-lease` if rebasing), and re-poll. Repeat until every check is green. |
|
|
||
| 1. **Bump the version** in `requirements.txt` (and `.circleci/config.yml` orbs / `Dockerfile` base image, if in scope for the task). | ||
| 2. **Regenerate the docs**, from the repo root: `./scripts/generateDocs.sh`. This requires Docker (for the Python/Sphinx step) and `sbt`/network access (for the Scala scraping step). Review the diff for new/removed/renamed pattern IDs in `docs/patterns.json` and `docs/description/`, and check whether `docs/tests/` needs new fixture files for newly added patterns (add one small `.py` sample per new pattern, following the naming convention `docs/tests/<PatternId>.py` used by existing fixtures). | ||
| 3. **Build the sbt project and format-check it**: `sbt "clean; set scalafmtUseIvy in ThisBuild := false; scalafmtCheckAll; Test / scalafmtCheck; scalafmtCheck; universal:stage"` (mirrors the CI `publish_docker_local` job). |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The sbt command includes redundant formatting checks. scalafmtCheckAll already covers all project configurations including tests.
| 3. **Build the sbt project and format-check it**: `sbt "clean; set scalafmtUseIvy in ThisBuild := false; scalafmtCheckAll; Test / scalafmtCheck; scalafmtCheck; universal:stage"` (mirrors the CI `publish_docker_local` job). | |
| 3. **Build the sbt project and format-check it**: `sbt "clean; set scalafmtUseIvy in ThisBuild := false; scalafmtCheckAll; universal:stage"` (mirrors the CI `publish_docker_local` job). |
2 participants
Summary
Test plan