From 40059fdef79d366c487352eabf2e79fe28f0d7d3 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 1 Sep 2026 14:46:10 -0700 Subject: [PATCH 1/2] chore(skills): teach babysit about cubic, not just Greptile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop only re-triggered `@greptile` after a push. Two bots review this repo, so a round would end with Greptile at 5/5 and cubic's threads still open against an earlier commit, their findings never re-checked against the fix. Adds what distinguishes them: cubic has no score and reports only inline threads, posts a fresh review per run rather than editing one comment in place, answers to `@cubic-dev-ai review this PR` rather than `@cubic`, and reviews the commit that was HEAD when its run started — so a stale thread is a reply-and-resolve, not a fix. Both are now re-triggered as separate comments and confirmed `pending` before the loop waits, since a trigger that silently failed is indistinguishable from a reviewer that has not started. Also promotes CI to a stop condition. A red required check is not clean no matter what either reviewer says, and the lint and audit jobs routinely catch what a local run does not. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/babysit/SKILL.md | 67 ++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index 84110c349f0..755d0700fee 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -1,6 +1,6 @@ --- name: babysit -description: Drive a PR to a clean review (Greptile 5/5, zero open threads) — ships if needed, keeps it mergeable against staging, triggers Greptile, fixes real findings, replies to and resolves every thread, and loops until clean +description: Drive a PR to a clean review (Greptile 5/5, zero open threads) — ships if needed, keeps it mergeable against staging, re-triggers both Greptile and cubic, fixes real findings, replies to and resolves every thread, and loops until clean --- # Babysit PRs @@ -8,7 +8,8 @@ description: Drive a PR to a clean review (Greptile 5/5, zero open threads) — Owns a PR end-to-end through review: ship it, wait for the automatic review round, and if it isn't already clean, drive fix → reply → resolve → re-review cycles until Greptile reports 5/5 and there are zero open comment threads, keeping the branch mergeable against staging along the -way. Designed to be run under `/loop` (no fixed interval — let it self-pace on review latency) +way. Two bots review this repo — Greptile and cubic — and they behave differently; see +"Two reviewers" below. Designed to be run under `/loop` (no fixed interval — let it self-pace on review latency) so it survives across multiple wakeups in the same session. ## When to use @@ -23,20 +24,46 @@ Needs a PR number. If none is given and there's no open PR for the current branc first (which includes the `origin/staging` sync check — see `.agents/skills/ship/SKILL.md`) to create one. +## Two reviewers + +Both post inline threads that count toward "clean", and they need re-triggering separately: + +| | Greptile (`greptile-apps`) | cubic (`cubic-dev-ai`) | +|---|---|---| +| Verdict | `Confidence Score: X/5` in a summary comment | no score — only inline threads | +| Summary comment | edited in place across rounds | fresh review per run | +| Re-trigger | `@greptile` | `@cubic-dev-ai review this PR` | +| Latency | 1–3 min | 1–3 min | + +Post **both** after every push, as two separate comments. Triggering only Greptile is the easy +mistake: the PR then shows 5/5 with cubic's threads still open from an earlier commit, and its +findings never get re-checked against the fix. + +`@cubic-dev-ai review this PR` is the documented wording — `@cubic` alone does not trigger it. + +cubic reviews the commit that was HEAD when its run started, so a thread can describe code the +next commit already changed. Before treating a cubic finding as real, check whether the current +HEAD still has the problem — a stale round is a reply-and-resolve, not a fix. + ## Definition of "clean" -Both must hold: +All three must hold: 1. The latest Greptile summary comment reports **Confidence Score: 5/5** -2. `reviewThreads` (GraphQL, see below) has **zero threads with `isResolved: false`** +2. `reviewThreads` (GraphQL, see below) has **zero threads with `isResolved: false`**, from + either bot +3. Required checks are passing — `gh pr checks ` shows no `fail` (a red CI run is not clean + no matter what the reviewers say, and lint/audit jobs routinely catch what local runs miss) Do not stop early on "no new comments this round" alone — a thread can be open from an earlier -round. Always check both conditions freshly after every push. +round, and cubic often lands its first threads a round after Greptile's. Always check all three +conditions freshly after every push. ## Loop 1. **Check current state** before doing anything, including whether the PR is still mergeable: ```bash gh pr view --json mergeable + gh pr checks | grep -v skipping gh pr view --json comments -q '[.comments[] | select(.author.login=="greptile-apps")] | last | .body' gh api graphql -f query=' query { repository(owner: "", name: "") { pullRequest(number: ) { @@ -51,16 +78,20 @@ round. Always check both conditions freshly after every push. stop yet: re-run the same query with `after: ""` and keep paging until `hasNextPage` is `false` before evaluating "clean." A PR with more than 50 threads is rare but stopping on a partial page would silently miss unresolved ones past the cutoff. - If `mergeable` is `CONFLICTING`, fix that first (step 2). Otherwise, if Greptile is 5/5 and - every thread across all pages has `isResolved: true`, stop — report the outcome (see + The thread query returns both bots' threads — check `author.login` per thread, don't assume + they're all Greptile's. + If `mergeable` is `CONFLICTING`, fix that first (step 2). If a check is failing, fix that too + — treat it exactly like a review finding. Otherwise, if Greptile is 5/5, every thread across + all pages has `isResolved: true`, and no check is failing, stop — report the outcome (see "Reporting" below) and skip the rest of this list. 2. **If the PR has a merge conflict**, merge `origin/staging`, resolve the conflicts, run the usual pre-push checks, push, and go to step 8 to re-trigger review. -3. **If no review has run yet** (fresh PR, no Greptile comments): Greptile usually runs - automatically on PR open — confirm via `gh pr checks ` (look for `Greptile Review`) and - wait for that first round before doing anything else. +3. **If no review has run yet** (fresh PR, no bot comments): both run automatically on PR open — + confirm via `gh pr checks ` (look for `Greptile Review` and `cubic · AI code reviewer`) and + wait for both before doing anything else. They finish at different times, so a PR that looks + clean because only one has reported is not clean yet. 4. **If a review round has landed and it isn't clean**: for every thread where `isResolved: false`, triage the finding on its own merits — this is the part that requires @@ -113,14 +144,19 @@ round. Always check both conditions freshly after every push. rounds; checking sync only before the push (step 6) and never after is how a bad push or a PR whose commit history quietly went stale between rounds goes unnoticed. -8. **Re-trigger review** by posting `@greptile` as its own PR comment: +8. **Re-trigger both reviewers**, each as its own PR comment — a combined comment does not + reliably trigger both: ```bash gh pr comment --body "@greptile" + gh pr comment --body "@cubic-dev-ai review this PR" ``` + Then confirm both actually picked it up before waiting — `gh pr checks ` should show + `Greptile Review` and `cubic · AI code reviewer` as `pending`. If one stayed `pass` from the + previous round, its trigger did not land; re-post that one. 9. **Wait for the new round**, then go back to step 1. Pace the wait with `ScheduleWakeup` using - a fallback delay of ~250–300s (Greptile typically takes 1–3 minutes) — never busy-poll - in a sleep loop. Pass the same `/loop babysit PR ` prompt on each wakeup so the loop + a fallback delay of ~300s — both bots take 1–3 minutes, and CI is usually the slowest of the + three — never busy-poll in a sleep loop. Pass the same `/loop babysit PR ` prompt on each wakeup so the loop resumes correctly. 10. **Stop conditions**: clean state reached (see above), or the same unresolved finding or @@ -130,7 +166,8 @@ round. Always check both conditions freshly after every push. ## Reporting When the loop ends, summarize: how many rounds it took, what was actually fixed (one line each), -what was pushed back on as a false positive and why, and the final Greptile score / thread count. +what was pushed back on as a false positive and why, and the final state — Greptile score, open +thread count across both bots, and CI status. ## Public-repo hygiene @@ -150,4 +187,6 @@ notification email. - Never fix a finding with a hacky workaround — if the clean fix isn't obvious, find the sibling pattern elsewhere in the codebase solving the same class of problem and match it. - Never silently drop a finding — every thread gets either a code fix or a reasoned reply. +- Never re-trigger only one reviewer. Both get a comment after every push, and both get confirmed + `pending` before you start waiting. - Always re-run the `/ship`-style sync check before every push in the loop, not just the first. From 6fe11553ce059eb6f25efed710aaa0b9df33ab73 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 1 Sep 2026 14:50:47 -0700 Subject: [PATCH 2/2] chore(skills): treat a pending check as unfinished, not as passing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the round this adds, both caught in review. The stop condition tested only for a failing check, so a check still running when both reviewers finished counted as clean and the loop reported the PR ready before CI had its say. It now requires every check to have finished and passed, and sends a pending one back to the wait step. The thread-attribution note pointed at `author.login` per thread, but `PullRequestReviewThread` has no author field — GitHub rejects the query with `Field 'author' doesn't exist on type 'PullRequestReviewThread'`. Identity lives on the comments, so it now names the real path and says not to lift the field to the thread level. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/babysit/SKILL.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index 755d0700fee..16f8cef5b32 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -51,8 +51,11 @@ All three must hold: 1. The latest Greptile summary comment reports **Confidence Score: 5/5** 2. `reviewThreads` (GraphQL, see below) has **zero threads with `isResolved: false`**, from either bot -3. Required checks are passing — `gh pr checks ` shows no `fail` (a red CI run is not clean - no matter what the reviewers say, and lint/audit jobs routinely catch what local runs miss) +3. Every check has **finished and passed** — `gh pr checks ` shows no `fail` *and* no + `pending`. A red run is not clean no matter what the reviewers say, and the lint/audit jobs + routinely catch what a local run misses. A `pending` one is not clean either: it has not + reported yet, and treating "not failing" as "passing" reports the PR clean before CI has + had its say. Wait for it — the step-10 stop condition covers a check that never settles. Do not stop early on "no new comments this round" alone — a thread can be open from an earlier round, and cubic often lands its first threads a round after Greptile's. Always check all three @@ -78,12 +81,14 @@ conditions freshly after every push. stop yet: re-run the same query with `after: ""` and keep paging until `hasNextPage` is `false` before evaluating "clean." A PR with more than 50 threads is rare but stopping on a partial page would silently miss unresolved ones past the cutoff. - The thread query returns both bots' threads — check `author.login` per thread, don't assume - they're all Greptile's. + The query returns both bots' threads. A `ReviewThread` has no author of its own — identity + lives on its comments, so read the opener's at `comments.nodes[0].author.login` and do not + add an `author` field at the thread level, which makes the query fail to compile. If `mergeable` is `CONFLICTING`, fix that first (step 2). If a check is failing, fix that too - — treat it exactly like a review finding. Otherwise, if Greptile is 5/5, every thread across - all pages has `isResolved: true`, and no check is failing, stop — report the outcome (see - "Reporting" below) and skip the rest of this list. + — treat it exactly like a review finding. If a check is still `pending`, do not evaluate + "clean" at all: go to step 9 and wait for it. Otherwise, if Greptile is 5/5, every thread + across all pages has `isResolved: true`, and every check has finished and passed, stop — + report the outcome (see "Reporting" below) and skip the rest of this list. 2. **If the PR has a merge conflict**, merge `origin/staging`, resolve the conflicts, run the usual pre-push checks, push, and go to step 8 to re-trigger review. @@ -167,7 +172,7 @@ conditions freshly after every push. When the loop ends, summarize: how many rounds it took, what was actually fixed (one line each), what was pushed back on as a false positive and why, and the final state — Greptile score, open -thread count across both bots, and CI status. +thread count across both bots, and whether every check finished and passed. ## Public-repo hygiene