Add Agent Playbook section for automated updates - #1012
Conversation
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The PR successfully adds an 'Agent Playbook' to the README.md, providing a structured guide for repository updates. Codacy results are up to standards, and no security or logic flaws were identified.
The primary feedback focuses on documentation architecture: the new playbook overlaps significantly with the existing 'Add plugin' and 'Update Version with Docs' sections. Consolidation is recommended to prevent documentation rot and ensure a single source of truth for AI and human contributors. Additionally, several technical nitpicks regarding Scala formatting commands and GitHub CLI usage should be addressed to improve the playbook's accuracy.
About this PR
- The 'Agent Playbook' creates substantial redundancy with the existing 'Add plugin' and 'Update Version with Docs' sections. To prevent documentation rot, consider consolidating these instructions into a single unified workflow.
Test suggestions
- Verify README.md renders correctly with the new markdown tables and code blocks.
- Verify all file paths mentioned in the playbook (e.g., scripts/doc_generate.sh, docs/patterns.json) actually exist in the repository structure.
- Verify that the instructions for adding a new plugin correctly reference the necessary changes in both Scala and Ruby generator files.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| ``` | ||
| Review the diff under `docs/description/` and `docs/patterns.json` for new/removed/renamed cops, and check `docs/patterns.json`'s `"version"` field reflects the new RuboCop version. | ||
| 4. **Update fixtures** under `docs/tests/` and `docs/multiple-tests/` if cop behavior/naming changed in a way that breaks existing expectations. | ||
| 5. **Build and format the Scala side**: `sbt "scalafmtCheckAll; test:scalafmt; scalafmt; universal:stage"`. |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Simplify the formatting command and use backticks for clarity.
| 5. **Build and format the Scala side**: `sbt "scalafmtCheckAll; test:scalafmt; scalafmt; universal:stage"`. | |
| 5. **Build and format the Scala side**: `sbt "scalafmtAll; universal:stage"`. |
| - You need to *require* the plugin, so they are injected on the *registry* when fetching the available Cops | ||
| - After the `bundle install` the documentation generator will fetch examples from the Cops of the added plugins | ||
|
|
||
| ## Agent Playbook: Updating This Repository End-to-End |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: This playbook is a great addition for maintaining the engine. To prevent documentation rot, consider consolidating the redundant 'Add plugin' and 'Update Version with Docs' sections into this new playbook. This ensures a single source of truth for both human contributors and AI agents.
| 8. **Iterate on failures**, re-running only the relevant test command after each fix. | ||
| 9. **Commit** the `Gemfile`/`Gemfile.lock` bump together with the regenerated `docs/` files and any Scala plugin-list changes in one change. | ||
| 10. **Push and open a PR.** CI (`.circleci/config.yml`) runs `codacy/checkout_and_version` -> `publish_docker_local` (scalafmt checks + `universal:stage` + `docker build`) -> `plugins_test` (`codacy_plugins_test/run`, multiple tests) -> `codacy/publish_docker` (master only) -> `codacy/tag_version`. | ||
| 11. **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 (the CircleCI job for the failing step — 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 (Ruby/gem resolution, JVM, sbt) can differ subtlely from your local one, so a clean local run does not guarantee CI passes — this repo's history shows follow-up "fix" commits landing right after a version-bump PR (e.g. a bump PR merged, then a same-day fix commit touching `.circleci/config.yml`, `Dockerfile`, `build.sbt`, and `Rubocop.scala` together) precisely because something only surfaced once real CI ran. 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.
⚪ LOW RISK
Nitpick: Correct the spelling of 'subtly' and simplify the gh command.
| 11. **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 (the CircleCI job for the failing step — 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 (Ruby/gem resolution, JVM, sbt) can differ subtlely from your local one, so a clean local run does not guarantee CI passes — this repo's history shows follow-up "fix" commits landing right after a version-bump PR (e.g. a bump PR merged, then a same-day fix commit touching `.circleci/config.yml`, `Dockerfile`, `build.sbt`, and `Rubocop.scala` together) precisely because something only surfaced once real CI ran. 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. | |
| 11. **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` and keep re-polling (short sleep while any check is `pending`) until all checks finish. If a check fails, fetch its actual log (the CircleCI job for the failing step — 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 (Ruby/gem resolution, JVM, sbt) can differ subtly from your local one, so a clean local run does not guarantee CI passes — this repo's history shows follow-up "fix" commits landing right after a version-bump PR (e.g. a bump PR merged, then a same-day fix commit touching `.circleci/config.yml`, `Dockerfile`, `build.sbt`, and `Rubocop.scala` together) precisely because something only surfaced once real CI ran. 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. |
| | `Gemfile` → `gem "rubocop"` (unpinned) and the `gem "rubocop-*"` plugin lines | Which RuboCop core/plugin versions `bundle update` will resolve to | RuboCop itself is **not version-pinned here** — the actual resolved version only shows up in `Gemfile.lock` after `bundle update`. If a plugin needs pinning to a specific version, add a version constraint on its line. | | ||
| | `Gemfile.lock` | The exact resolved RuboCop + plugin + transitive dependency versions actually shipped | Regenerated by `bundle update` / `bundle install` — do not hand-edit; commit the regenerated file. | | ||
| | `docs/patterns.json` → top-level `"version"` field | The RuboCop version recorded in the generated pattern catalog | Set automatically by the doc generator when re-run after the gem bump — verify it matches the new `Gemfile.lock` version after regeneration. | | ||
| | `src/main/scala/codacy/rubocop/Rubocop.scala` → `plugins` list (and the "legacy requires" block) | Which cop plugins are `require`d/enabled when the engine invokes RuboCop | Add new plugin gems here (mirrors the `doc_generation/codacy/rubocop/generator.rb` `$plugins` list — keep both in sync, see the "Add plugin" section above). | |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The playbook refers to an 'Add plugin' section above, but no header with this exact name exists. Consider adding a header or updating the reference to point to the relevant instructions (currently appearing as bullet points around line 39).
Summary
Test plan