From 07c5dffab0097bcd072fe971f48748206681166b Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sat, 3 Oct 2026 19:35:31 +0200 Subject: [PATCH] docs(agents): review locally with Codex and Greptile before PR pushes (#1801) Every push to a pull-request branch starts the CI matrix and both review bots, and runs from several open pull requests queue behind one another. Move the fix rounds off GitHub: a branch is reviewed with the Codex and Greptile CLIs until both are clean, then pushed once. - AGENTS.md: the rule, linked to the procedure - agent-workflow.md: commands, loop, stop conditions and exemptions - agent-context-map.md: route the topic to the workflow document Co-authored-by: Claude Fable 5.1 --- AGENTS.md | 6 ++++ docs/development/agent-workflow.md | 42 +++++++++++++++++++++++++++ docs/maintenance/agent-context-map.md | 2 +- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 3119f7d6a..fd52f65de 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,6 +54,12 @@ contracts below before changing a subsystem; do not load every document. `no-release-note` on exempt PRs touching runtime code. - Validate notes with `pnpm run release:notes:validate`. Release publication has separate ordered gates; follow the release-cut skill and release contract. +- Before the first push of a pull-request branch, and before each later push to + an open pull request, pass the + [local review gate](docs/development/agent-workflow.md#local-review-before-a-pull-request): + Codex and Greptile CLI reviews of the committed branch, repeated until both + are clean. CI and the GitHub review bots confirm a branch; they are not its + first reviewer. ## Keep guidance small and canonical diff --git a/docs/development/agent-workflow.md b/docs/development/agent-workflow.md index 239feed90..3e7fbe4bb 100644 --- a/docs/development/agent-workflow.md +++ b/docs/development/agent-workflow.md @@ -134,6 +134,48 @@ Completion reports list changed docs, tests added/updated, commands and results, skipped validation with reasons, and release-note status. A docs-only task needs Markdown/link validation, not app unit/E2E tests. Tooling changes need their own tests. +## Local review before a pull request + +Every push to a pull-request branch starts the whole CI matrix and both review +bots, and runs from several open pull requests queue behind one another. Fix +rounds therefore happen locally: push only a commit that both reviewers have +already accepted. + +Greptile reviews committed work only, so commit first and fetch the base; both +reviewers then judge the same change. Run them from the branch's worktree and +keep their output outside the repository: + +- Codex: `codex review --base origin/master -c model_reasoning_effort=high`. + The verdict is the final message on stdout; stderr carries the session log. + The flag pins the review effort whatever the caller's default is. Codex + otherwise runs with the caller's own configuration and may install + dependencies or run tests in the worktree, so do not start other installs or + builds there while it reviews. +- Greptile: `greptile review --json`. Clean means `confidence` is 5 and + `comments` is empty. A non-zero exit means the review did not run, not that + it found problems. + +1. Finish the change and its targeted checks, then commit. +2. Run both reviewers on the same commit; they are independent and can run in + parallel. +3. Check every finding against the code. Fix the real ones and note a one-line + reason for each one declined. Never clear a finding by suppressing a rule or + weakening a test. +4. Commit the fixes and review again. Stop when both reviewers are clean on the + same commit. Also stop after five rounds, or when a round repeats the + previous findings, and report what remains instead of pushing. +5. Push, open the pull request, and name the reviewed commit and any declined + findings in the completion report. + +The GitHub bots still review the opened pull request. Treat their findings and +CI failures the same way: collect the whole round, fix it locally, pass both +local reviewers again and push once. Do not push one fix per finding. + +A reviewer whose CLI is missing, signed out, outside a Greptile organization or +left without reviewable files (Greptile ignores Markdown-only changes) does not +block the other one. Say which reviewer did not run; do not install a CLI, sign +in or onboard an account on the maintainer's behalf. + ## Repository skills Repository skills live under `.codex/skills/`. Descriptions are trigger-only, diff --git a/docs/maintenance/agent-context-map.md b/docs/maintenance/agent-context-map.md index fe599c138..cc49ce265 100644 --- a/docs/maintenance/agent-context-map.md +++ b/docs/maintenance/agent-context-map.md @@ -12,7 +12,7 @@ are not prerequisites for reading repository contracts. | Area / code ownership | Canonical documents | Repository skill | | --- | --- | --- | | Bootstrap, project placement, dependencies, aliases and lint configuration; root Nx config and project-local project.json files | [Nx boundaries](../architecture/nx-workspace-boundaries.md), [security overrides](../architecture/dependency-security-overrides.md) | [Nx architecture](../../.codex/skills/iptvnator-nx-architecture/SKILL.md) | -| Angular conventions; docs and skills maintenance | [Agent workflow](../development/agent-workflow.md) | Use the area's skill below | +| Angular conventions; docs and skills maintenance; local review before a pull request | [Agent workflow](../development/agent-workflow.md) | Use the area's skill below | | Unit, E2E, lint and coverage; `tools/coverage`, `tools/typecheck` | [Validation map](../architecture/validation-map.md) | Use the area's validation section | | Performance journeys, counters, benchmark probes and the CI ratchet; `apps/electron-backend-e2e/src/journeys`, `apps/electron-backend-e2e/src/performance`, `tools/performance` | [Performance journeys](../architecture/performance-journeys.md) | Read the contract directly | | Electron entry/events/preload and CDP; `apps/electron-backend` | [Debugging and trace flags](../development/electron-debugging.md), [Electron security](../architecture/electron-security.md) | Use the available global electron skill for automation |