Repository navigation
feat(babysit): add release threshold for review findings - #13
Conversation
…g merges Babysit runs treated every valid review finding as a merge blocker and had no way to record a deferral, so Rukoo #2 ran 14 Codex rounds (54 P2, 10 P1) and optimal-hermes #1 ran 12 before the user stepped in. Helper: - Read the Codex P0-P3 badge for each thread, only when a configured reviewer opened it. No other comment text is kept. - Add `decide --decision deferred`, with batch --finding. P0, P1, and unrated findings need --user-approved, recorded only on those findings. - Report codeReview.rounds and `next` actions, including a head-aware prompt to request review of a repaired head. - Show open findings by severity, deferrals, and rounds in wake messages. Skill: - Severity triage table: P0/P1 block; P2/P3 are deferred unless a commit is already going out for another reason. - Round budget: ask the user before pushing after a fourth review that still raises new P0/P1 findings. - List the only merge gates; optional checks and local reviews are not gates. - Request re-review in the same step as each push, run inspect after pushes, merge without asking again, one agent per branch.
Mira PR WalkthroughThis PR adds P0–P3 severity reporting and a release threshold that allows P2/P3 findings to be deferred without blocking readiness. Deferring P0, P1, or unrated findings requires recorded user approval, with support for atomic decisions across multiple review threads. It also adds review round counts and next-step guidance to inspection and wake reports, updates documentation, and adds threshold and deferral coverage. graph LR
github["skills/babysit/scripts/github.mjs"] --> inspect["skills/babysit/scripts/inspect.mjs"]
inspect --> babysit["skills/babysit/scripts/babysit.mjs"]
inspect --> wake["skills/babysit/scripts/wake.mjs"]
Confidence: 3/5 ◉◉◉○○ Needs validation
Key files to review:
9 files reviewed · 1 comment (
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![clark-review[bot]](https://avatars.githubusercontent.com/u/8057288?s=60&v=4){kind=small}
There was a problem hiding this comment.
Mira Review Summary
skills/babysit/scripts/github.mjs recognizes a priority badge only at the start of a finding, so embedded or quoted badges can misclassify unrated findings as P2/P3. This can let those findings be deferred without the required approval.
Key Issues
| Issue | Location | |
|---|---|---|
| 🔴 | Embedded or quoted badges can classify unrated findings as P2/P3 and bypass the deferral approval requirement. | skills/babysit/scripts/github.mjs:205 |
| * Only this token is kept, never the text around it. | ||
| */ | ||
| export function reviewSeverity(body) { | ||
| const match = /!\[(P[0-3]) Badge\]/u.exec(String(body ?? "").slice(0, 300)); |
There was a problem hiding this comment.
Bug
Only recognize a priority badge at the start of the finding
The regex accepts a badge anywhere in the first 300 characters, including quoted examples or code snippets. For example, a configured reviewer's unrated comment starting with Example: \`
becomes P2.findingSeveritytrusts this token, anddecide --decision deferred` then discharges the finding without user approval. Match only the opening priority badge, allowing the Markdown/HTML wrappers used by actual reviewer output, and add regression coverage for quoted and embedded badges.
Prompt for AI Agents
Update reviewSeverity in skills/babysit/scripts/github.mjs at line 205 to recognize only an opening priority badge, allowing the Markdown and HTML wrappers in the existing Codex badge fixture. Reject badges embedded in prose, quotations, and code examples. Add regression cases in tests/babysit/threshold.test.mjs showing these comments remain unrated and require user approval for deferral.
Not useful? Reply
@miracodeai rejectto dismiss this suggestion.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3866bc8efb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const entry = { | ||
| threadId: thread.id, | ||
| author: first?.author ?? null, | ||
| severity: findingSeverity(thread, policy), |
There was a problem hiding this comment.
When a P2/P3 finding is deferred and init is later rerun with a different --code-reviewer, the thread becomes unrated under the current policy, but classification still accepts the old decision solely because its threadVersion matches. The PR can therefore remain ready without the --user-approved authorization now required for that finding; deferred decisions should be invalidated or reauthorized when their recorded severity no longer matches the current severity.
Useful? React with 👍 / 👎.
| const TRIAGE = | ||
| "Triage open findings by severity: fix P0/P1; fix a P2/P3 only when you already push a commit for another reason and the fix is small and safe, otherwise decide --decision deferred."; |
There was a problem hiding this comment.
When codeReview.rounds reaches four and the latest review still has an open P0/P1, nextActions continues to emit the unconditional instruction to fix it. This conflicts with the newly documented round budget, which requires stopping and asking the user before another push, and SKILL.md explicitly tells agents to perform the reported next actions. Include the round count and open severities when selecting this action so the wake directs the agent to escalate instead of beginning a fifth repair cycle.
Useful? React with 👍 / 👎.
reviewSeverity accepted a priority badge anywhere in the first 300 characters, so a reviewer comment that quoted `` became P2 and could be deferred without the user's approval. Match only a badge at the start of the comment, inside Codex's bold and <sub> wrappers. classify kept a deferral as long as the thread text was unchanged. After `init` changed the reviewer, a deferred P2 became unrated but stayed discharged. A deferral now holds only while it would still be allowed: the finding is P2/P3, or the user approved deferring it at its current severity. Otherwise it reopens.
|
@codex review |
|
Triage of the first review round, fixed in fb1a3ca:
Both have regression tests in Deferred, non-blocking:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb1a3ca204
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| findings: { open, discharged }, | ||
| tested: testedView(state, head), | ||
| next: nextActions(reasons, staleReviews.length > 0), |
There was a problem hiding this comment.
When a clean review arrives for an older commit while the current head remains unreviewed, staleReviews.length changes this action from the opening-review guidance to the repaired-head instruction to request @codex review, but summary contains neither next nor stale reviews. Because recordObservation suppresses wakes when the summary-derived newsKey is unchanged, a durable watcher emits no event in this scenario and the agent can continue waiting indefinitely instead of requesting review of the current head; include the action-driving state in the summary/news key.
Useful? React with 👍 / 👎.
| const TRIAGE = | ||
| "Triage open findings by severity: fix P0/P1; fix a P2/P3 only when you already push a commit for another reason and the fix is small and safe, otherwise decide --decision deferred."; |
There was a problem hiding this comment.
When the only open finding is unrated, or a P0/P1 is a false positive, this next action provides no applicable route to clear the gate: it mentions only fixing P0/P1 and fixing or deferring P2/P3, despite decide supporting refutation and requiring user approval to defer unrated findings. Since the skill directs agents to perform reported next actions before yielding, the guidance can stall on an unrated finding or prompt an unnecessary code change for a refutable blocking finding; explicitly cover refutation and the user-approved deferral path for P0/P1 and unrated findings.
Useful? React with 👍 / 👎.
|
Second review round, at fb1a3ca: two P2 findings, both deferred as non-blocking.
Follow-up for a later PR: make |
1 participant
Summary
Testing