# Pr Cycle > PR Cycle (otari) - Skill: `mozilla-ai/pr-cycle` (Agent Skill) - Install (CLI): `npx skillmds@latest add mozilla-ai/pr-cycle` - Raw SKILL.md: https://api.skillmd.com/api/skills/mozilla-ai/pr-cycle/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: mozilla-ai (https://skillmd.com/u/mozilla-ai) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/mozilla-ai/pr-cycle --- # PR Cycle (otari) Drive an issue from code to a reviewed, ready PR without hand-holding. One issue = one branch = one PR. For several issues, fan out (see [Multi-issue orchestration](#multi-issue-orchestration)). Compose with the sibling skills rather than repeating them: [`review`](../review/SKILL.md) for the self-review, [`frontend-standards`](../frontend-standards/SKILL.md) before any `web/` edit (its [testing.md](../frontend-standards/testing.md) is the dashboard's test guidance), and [`backend-standards`](../backend-standards/SKILL.md) before any `src/gateway/` edit (its "Before you finish" section carries the backend test bar). [AGENTS.md](../../../AGENTS.md) owns the repo-wide facts this skill leans on: the two runtime modes, the test notes, and the generated artifacts. ## The cycle (one PR) 1. **Branch** off `origin/main` (`git fetch origin main` first). Name it `/` (`fix/…`, `perf/…`, `refactor/…`, `docs/…`). Never push to `main`. 2. **Understand the issue.** Read it and the exact files and lines it names, and verify current signatures before editing rather than trusting the issue's snippet. Read `AGENTS.md` plus the scoped one for the directory you are in (`web/`, `src/gateway/`). A backend edit respects the layer rules `scripts/check_architecture.py` enforces; a management route is standalone-only, so check `register_routers()` placement. 3. **Implement.** For a refactor, keep it strictly **behavior-preserving**: no change to a public API, a route, a query key, or an invalidation. Correctness over speed. 4. **Tests.** Happy path and error path, next to the behavior they cover (unit for pure logic, integration for route or database behavior). Mock the dashboard at the `apiFetch` boundary, not the hooks. Never weaken coverage, and never add a global pytest rerun policy: a genuinely flaky test carries `@pytest.mark.flaky(reruns=...)` and a stated reason. 5. **Checks** must be green before opening (see [Running checks](#running-checks)), and **regenerate whatever the change made stale** (see [Generated artifacts](#generated-artifacts-a-pr-can-owe)). 6. **Commit.** Conventional Commits. End every message with a `Co-Authored-By:` trailer naming the model that wrote it (`Co-Authored-By: `). Fill in your own identity; do not copy a model name out of an example or another commit. 7. **Open the PR** against `mozilla-ai/otari` with `Fixes #`. The **title must be a Conventional Commit** (`otari-pr-title.yml` gates it; accepted types are `feat`, `fix`, `perf`, `security`, `revert`, `chore`, `build`, `ci`, `docs`, `style`, `refactor`, `test`), because the repo squash-merges and git-cliff parses that title into the changelog. Keep the template's `## PR Type`, `## Checklist` and `## AI Usage` sections: `pr-template-check.yml` fails and labels the PR `missing-template` if any of the three is absent. Fill in AI Usage honestly, including the AI-agent checkbox. Two further sections are expected on every PR and are gated by nobody, which makes them the ones an agent drops: `## Description` in plain English for a reader with no context on the area (what changes for someone using Otari, and why, no file paths), and `## How to test it locally` with the steps a reviewer runs plus the automated checks that already cover it. No labels are required here. No em dashes in the description (repo prose rule). Default to opening **ready for review**; open a **draft** only if the user asked to see it first (confirm which if unsure). 8. **Self-review.** Invoke the [`review`](../review/SKILL.md) skill on your own PR before anyone else reads it. Apply what is valid and push; skip nits that fight the repo's conventions, and say why. 9. **Request reviewers** (see [Requesting reviewers](#requesting-reviewers)). 10. **Wait for the review, then fix** (see [Handling the review](#handling-the-review)). Leave the PR ready (or draft, per step 7). **Never merge unless told to**, and note that `main` is protected as well (see [Merging](#merging-when-asked-to-merge)), so a merge needs a human approval on top of your instruction. ## Running checks From the repo root: - `make lint`: the architecture check, then Ruff. **Ruff alone is not equivalent.** A layer violation fails here with a clean `ruff check`. - `make typecheck`: mypy. - `make test`: `tests/unit` and `tests/integration`. `make test-unit` and `make test-integration` split it while iterating. Integration tests need PostgreSQL: `TEST_DATABASE_URL` when set, otherwise a Testcontainers `postgres:17`. Whichever it is, it is a *server* URL: each xdist worker creates a database of its own on it (`postgres` becomes `postgres_gw0`, and so on) and drops it at the end of the session, so the credentials need `CREATE DATABASE` and a `postgres` database to connect through. With no Docker, point `TEST_DATABASE_URL` at any reachable instance; SQLite is not a fallback, because none of that is available there. Two tests make a real outbound call and report a status mismatch with no network egress (`test_error_detail_leakage.py::test_provider_error_does_not_leak_details`, `test_streaming_error_event.py::test_streaming_creation_error_returns_http_error`): that is environment noise, not a regression, so confirm the change against the rest of the suite. A change to the app, the migrations, or dependency resolution also owes the OSS-edition smoke gate: `uv run --frozen --no-dev python scripts/oss_edition_smoke.py`. It defaults to a throwaway SQLite file, so it needs no Docker. The dashboard has its own, which `make lint` does not touch: `pnpm --dir web run lint`, `pnpm --dir web run typecheck`, `pnpm --dir web test`. Screenshot baselines are gitignored and that suite runs on demand, so a PR that moves a page owes no PNGs; a PR that **adds** a page owes a screenshot entry so the page is covered when the suite becomes a gate. ## What CI runs on a PR, and the two ways it silently does not Every substantive workflow but one is declared `pull_request: branches: [ main ]` (`otari-dashboard.yml`, `otari-dashboard-parity.yml`, `otari-design-system.yml`, `otari-tests.yml`), and that filter is on the **base** branch. The exception is `otari-sdk-codegen-check.yml`, which declares `pull_request` with a `paths` filter and no `branches` key at all, so it matches any base: a stacked PR touching `docs/public/openapi.json` or `scripts/sdk_codegen/**` runs its four-language generate matrix and can go red where this section otherwise promises nothing. Two situations therefore leave a PR with almost no CI, and neither of them reports anything: - **A base that is not `main`.** A PR stacked on another feature branch matches no workflow, so it gets only the `pull_request_target` checks. Retargeting a stacked PR onto its parent branch is what usually causes this, and the trade is invisible in the direction you are looking: the diff gets smaller and the check count drops from seven to one or two. Both ends of that range depend on when you look, and it is worth knowing why: `otari-pr-title.yml` lists `synchronize`, so `Lint PR title` re-runs on every push and is attached to the current head, while `pr-template-check.yml` lists only `opened` and `edited`, so `check-template` stays attached to the sha the PR was opened (or last edited) at. So the ceiling is seven on that sha and six on every head after a push, and the floor is two and then one. Measured on four stacked heads: one row each, `Lint PR title`, with no `check-template` on any of them. Keep a stacked PR on base `main` and live with the inherited diff until its parent merges. - **A `CONFLICTING` PR.** A `pull_request` workflow builds the PR's merge ref, and a conflicting PR has none, so nothing runs until the conflict is resolved. The tell is `mergeStateStatus: DIRTY` beside a check count that has stopped growing. A child PR hits the second case as soon as its parent is squash-merged, because its own unsquashed commits and the squash on `main` are the same content twice. `git rebase --onto origin/main ` drops the absorbed commits and clears it. **"Green" means the full expected set is present and none of it is pending**, which is not the same as nothing being pending. Both situations above produce a `gh pr checks` that lists one or two passing rows, and a passing row reads as a clean result to anyone who does not already know what the set should be. Check the names rather than only the buckets: on a dashboard change the substantive ones are `build`, `dashboard`, `e2e`, `catalog` and `serving`, and a run without them has covered nothing. A small set is not always one of the two faults above, though. Every one of these workflows also carries a `paths` filter, so a change that touches nothing they watch correctly runs almost nothing: a PR editing only `.github/skills/`, or a `docs/` file other than `dashboard.md` and `public/openapi.json`, gets the title and template checks and no more, and that is the right answer rather than a symptom. Those two `docs/` paths are the exception, watched by `otari-dashboard.yml`, `otari-dashboard-parity.yml`, `otari-dashboard-serving.yml` and `otari-docker-build.yml` because the dashboard bundles the guide and generates its client from the spec, so a one-line edit to either runs `dashboard`, `e2e`, `serving` and `build`. The question to ask is whether the set matches the change, not whether the set is large. ## Generated artifacts a PR can owe - A route, a schema, **or a route docstring**: run `uv run python scripts/generate_openapi.py`, then `make postman`, and commit both. A single CI job runs `make openapi-check` and `make postman-check`, so regenerating only the spec still fails. - `web/src/client/schema.ts` and `web/src/routeTree.gen.ts` are committed and drift-checked. The dashboard bundle (`src/gateway/static/dashboard/`) is not committed, so a `web/src` change sometimes leaves a file to commit and never leaves a bundle to commit. - Never hand-edit `CHANGELOG.md`. The release workflows regenerate it from the squashed titles. ## Requesting reviewers **One bot reviews this repo.** **CodeRabbit** reviews automatically, without being requested, and leaves inline comments of its own; address its threads the way you would a person's (reply, then resolve). Nothing has to be requested to get it. **Copilot is gone.** The org gave up its access in September 2026, so do not request it. The attempt fails silently rather than erroring: `POST /pulls//requested_reviewers` returns `201 Created` and GraphQL `requestReviews` returns success, both while adding no reviewer, so a poll for its review waits out the timeout on a bot that was never coming. - **Team.** `gh pr edit --add-reviewer mozilla-ai/otari-team`. CODEOWNERS auto-requests that team only on the open-core guardrail paths (`ARCHITECTURE.md`, `scripts/check_architecture.py`, `.github/CODEOWNERS`), so every other PR needs the request made explicitly. ## Handling the review 1. **Wait for CodeRabbit to finish.** It appears under `/pulls//reviews` with state `COMMENTED` once done. It is not instant, and its summary comment can land before the inline threads do, so a review that looks empty may not be finished. 2. **Read the comments.** The summary body is in `/pulls//reviews`. **Inline comments can be missing from the REST `/pulls//comments` endpoint for bot reviews** even when the summary claims N comments, so fetch the threads over GraphQL: ```bash gh api graphql -f query='{ repository(owner:"mozilla-ai",name:"otari"){ pullRequest(number:){ reviewThreads(first:50){ nodes{ isResolved isOutdated id comments(first:10){ nodes{ databaseId author{login} path line originalLine body } } } } } } }' ``` 3. **Triage.** Apply every valid finding; skip a nit that contradicts an established convention here and say so. Verify against current source before "re-fixing" anything: GitHub re-anchors comments to HEAD, so an addressed comment can look like it came back. **When a finding is about a claim, grep the repo for the claim, not the file.** The habit worth correcting here is fixing the place the problem was noticed rather than every place it lives. One review caught the same stale statement in three files, a role whose color was checked and whose metrics were not, and one of two identical hand-rolled values in a file already being edited. Each fix was right and each search was one file too narrow, so after applying a finding, search for the pattern rather than re-reading the diff. 4. **Push** the fixes. On each addressed thread, **reply** with one line on how it was addressed, then **resolve** it: ```bash # reply: databaseId comes from the query above gh api -X POST repos/mozilla-ai/otari/pulls//comments//replies \ -f body='Addressed in : .' # resolve: GraphQL only, with the thread's node id (there is no REST endpoint) gh api graphql -f query='mutation{ resolveReviewThread(input:{threadId:""}){ thread{ isResolved } } }' ``` A reply to a resolved thread does not reopen it. Keep replies terse and free of em dashes: it is a poor look to trip the prose rule in the same breath as answering a review. 5. **Resolve your own self-review threads too.** `required_review_thread_resolution` counts every thread, including the ones you opened on your own diff in step 8. Explanatory comments that need no action still block the merge until resolved, which is a non-obvious cost of doing the self-review properly. Resolve them once they have been read, the same way as a reviewer's. 6. Leave the PR ready (or draft, per the original decision). ## Multi-issue orchestration To turn a batch of issues into PRs at once, run **one worktree-isolated agent per issue** (Agent tool, `isolation: "worktree"`), each executing the cycle above. - **Group into waves by file independence.** Issues touching disjoint files run in parallel; issues sharing a file are **sequenced**, with the later ones rebasing once the earlier lands. - **Order within a wave:** small and low-risk first, large refactors later, so the rebase surface stays small. - **A stacked PR loses three protections, so stack deliberately.** Basing a PR on a topic branch instead of `main` buys a clean diff (only your own commits, not the parent's) and costs the things that would otherwise catch a mistake. CI is the third and it has its own section above ("What CI runs on a PR"); the other two: `protect-main` applies to the default branch only, so the child reports `mergeStateStatus: CLEAN` and can be merged into its base with **no approval and no thread resolution**, silently folding two reviews into one. And `.coderabbit.yaml` sets `base_branches: ["main"]`, so **CodeRabbit skips the child entirely** (it posts a "Review skipped" comment saying so); trigger it with a `@coderabbitai review` comment. Neither is a reason not to stack, and both are reasons to say in the PR body that it is stacked and what has to happen before the parent merges. - **Branch shape.** Squash and rebase merges are enabled and **merge commits are disabled**, so a branch collapses to one commit on `main` and its internal shape never lands. Still update with `git rebase origin/main` and `git push --force-with-lease` rather than merging `main` in: the PR diff and CI stay about your change. - **Poll for reviews centrally**, not inside each agent, since an agent idling on a review burns its run for nothing. Once a wave's PRs are open, poll until each has a completed CodeRabbit review, then route the fixes back to the original agents with `SendMessage` (their worktree and context are intact) rather than starting fresh ones. - **Resource note.** Each agent running the integration suite boots its own Testcontainers Postgres. Stagger them rather than sharing one `TEST_DATABASE_URL`: two suites running at once against one server pick the same worker database names and drop each other's database out from under the run. Sharing a server is only safe if each agent gets a distinct database in the URL. ## Merging (when asked to merge) - **Squash is the button** (merge commits are disabled). The PR title becomes the commit on `main` and the changelog line, so it has to be the sentence you want released. - **Auto-merge is enabled** on the repo, so `gh pr merge --squash --auto` works while CI runs. - **`main` is protected, by a ruleset rather than by legacy branch protection.** This matters because of how it looks from the API: `gh api repos/mozilla-ai/otari/branches/main/protection` returns **404 "Branch not protected"**, which is not evidence of no protection. It only means the rules are not the legacy kind. Read `gh api repos/mozilla-ai/otari/rulesets` instead, or `gh api repos/mozilla-ai/otari/branches/main -q .protected`, which reports `true`. An earlier version of this file claimed `main` was unprotected on the strength of that 404. The `protect-main` ruleset requires, on the default branch: - **one approving review**, and a bot's `COMMENTED` review does not satisfy it. CodeRabbit comments rather than approves, so a PR with its threads resolved and green CI still reports `mergeStateStatus: BLOCKED` and `reviewDecision: REVIEW_REQUIRED`. - **every review thread resolved** (`required_review_thread_resolution`). See the warning in [Handling the review](#handling-the-review) about your own self-review counting here. - **an extra approval for unattributed changes** (`require_extra_approval_for_unattributed_changes`). It keys on whether GitHub can attribute every commit in the PR to a user account, so a commit whose author email is not linked to one trips it. That matters here more than anywhere else in this file, because the subject is agents pushing commits: it presents as "green CI, one approval, still `BLOCKED`", which sends the reader back to the API. Avoid it by committing with an email linked to the account, rather than discovering it. - squash or rebase only, plus `deletion` and `non_fast_forward` rules on the branch. Repository admins are bypass actors, so an admin *can* merge through all of it. That makes the bar the one you hold yourself, unchanged: green CI, the review addressed, a human approval, and an explicit instruction to merge. - **Branches are deleted on merge**, which makes the stacked-PR trap real. Merging parent A with its branch deleted **closes child B permanently**: B's base is gone and GitHub refuses to reopen a PR whose base branch was deleted (`422 "state cannot be changed"`), even if you recreate the branch. So `gh pr edit B --base main` **while A's branch still exists**, then merge A. Base `main` is also what gets B any CI at all (see [What CI runs on a PR](#what-ci-runs-on-a-pr-and-the-two-ways-it-silently-does-not)), so it is where a stacked PR should have been sitting all along. If B already closed this way, the only recovery is a fresh PR from B's head branch, linking the old one for its review history. ## Non-negotiables - Never push to `main`; never merge unless told; confirm before any other outward-facing action, including posting a review or commenting on somebody else's PR. - Every PR: `Fixes #`, a Conventional-Commit title, the three template sections, and commit messages carrying a `Co-Authored-By:` trailer for the model that wrote them. - A behavior-preserving refactor changes structure, not observable behavior. That is the bar. - `make lint` before declaring done, not `ruff check`: the architecture check runs first and is the half that catches a layer violation.