Skip to content

Commit d554d17

Browse files
indrasvatindrasvat
committed
fix(review): scope completion signals to codex
1 parent 1b57115 commit d554d17

7 files changed

Lines changed: 124 additions & 61 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,11 @@ With `--solo`, the approval requirement is skipped (but `CHANGES_REQUESTED` stil
170170
Stale `CHANGES_REQUESTED` reviews still block until explicitly dismissed. `status` surfaces them in
171171
`stale_reviews` and suggests a safe `gh ghent dismiss` command.
172172

173-
`--await-review` watches PR-level review signals as well as threads and reviews. A Codex-style
174-
`eyes` marker keeps the review wait open; a Codex-style `thumbs up` completion marker can end
175-
the wait early, after which `status` still performs the normal full threads/checks/reviews fetch.
173+
`--await-review` watches Codex-owned PR-level review signals as well as threads and reviews.
174+
On repos with Codex enabled, an `eyes` marker keeps the review wait open; a Codex-owned
175+
`thumbs up` completion marker can end the wait early, after which `status` still performs
176+
the normal full threads/checks/reviews fetch. Repos without Codex keep the conservative
177+
thread/review polling behavior.
176178

177179
Exit codes: `0` = merge-ready, `1` = not merge-ready.
178180

‎docs/LEARNINGS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
- **2026-03-25 (await-review):** Debounce on zero activity is wrong for review-await — a bot may take 2-4 min before posting its first comment (Codex shows 👀 during this time). Only debounce after at least one fingerprint change; use hard timeout as the safety valve for zero-activity cases
2323
- **2026-03-25 (await-review):** When sorting parallel slices (IDs + metadata), sort as struct entries — `sort.Strings(ids)` alone mis-pairs the metadata slices. Codex P1 caught this.
2424
- **2026-03-25 (await-review):** Take a baseline activity fingerprint BEFORE CI watch starts, not after — activity that happens during CI (fast bot reviews) is invisible if the initial probe is taken post-CI. Compare baseline vs post-CI to detect it.
25-
- **2026-05-17 (await-review):** Codex PR-body status markers are useful but asymmetric signals: eyes means "do not settle yet"; thumbs up can fast-settle the review-await phase only when no unresolved threads are visible and the final full status fetch still gates merge readiness.
25+
- **2026-05-17 (await-review):** Codex PR-body status markers are useful but asymmetric signals: eyes means "do not settle yet"; thumbs up can fast-settle the review-await phase only when the PR body editor is Codex-like, no unresolved threads are visible, and the final full status fetch still gates merge readiness. Repos without Codex must remain on conservative thread/review polling.
2626
- **2026-03-26 (bot-sweep):** GitHub GraphQL `author { __typename }` returns `"Bot"` for all GitHub App bots — this is the authoritative bot detection signal, superior to login-string matching. GraphQL author.login omits the `[bot]` suffix that REST includes.
2727
- **2026-03-26 (bot-sweep):** GitHub's `resolveReviewThread` mutation is idempotent — resolving an already-resolved thread succeeds silently. No special "already resolved" handling needed.
2828
- **2026-03-26 (bot-sweep):** Compute merge-readiness BEFORE applying display filters (--bots-only) — filtering mutates thread counts, which would make the PR appear merge-ready when unresolved human threads are hidden.

‎internal/domain/types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ type ActivitySnapshot struct {
316316
HeadSHA string `json:"head_sha"`
317317
PRUpdatedAt time.Time `json:"pr_updated_at,omitempty"`
318318
PRLastEditedAt time.Time `json:"pr_last_edited_at,omitempty"`
319+
PREditorLogin string `json:"pr_editor_login,omitempty"`
320+
PREditorType string `json:"pr_editor_type,omitempty"`
319321
PRReviewSignal PRReviewSignal `json:"pr_review_signal,omitempty"`
320322
ReviewDecision string `json:"review_decision,omitempty"`
321323
ThreadCount int `json:"thread_count"`

‎internal/github/activity.go‎

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ query($owner: String!, $repo: String!, $pr: Int!) {
2424
body
2525
updatedAt
2626
lastEditedAt
27+
editor {
28+
__typename
29+
login
30+
}
2731
reviewDecision
2832
reactionGroups {
2933
content
@@ -67,11 +71,15 @@ query($owner: String!, $repo: String!, $pr: Int!) {
6771
type activityResponse struct {
6872
Repository struct {
6973
PullRequest *struct {
70-
HeadRefOid string `json:"headRefOid"`
71-
Body string `json:"body"`
72-
UpdatedAt string `json:"updatedAt"`
73-
LastEditedAt *string `json:"lastEditedAt"`
74-
ReviewDecision string `json:"reviewDecision"`
74+
HeadRefOid string `json:"headRefOid"`
75+
Body string `json:"body"`
76+
UpdatedAt string `json:"updatedAt"`
77+
LastEditedAt *string `json:"lastEditedAt"`
78+
Editor *struct {
79+
TypeName string `json:"__typename"`
80+
Login string `json:"login"`
81+
} `json:"editor"`
82+
ReviewDecision string `json:"reviewDecision"`
7583
ReactionGroups []struct {
7684
Content string `json:"content"`
7785
Reactors struct {
@@ -217,9 +225,13 @@ func (c *Client) ProbeActivity(ctx context.Context, owner, repo string, pr int)
217225
ThreadCount: pr_.ReviewThreads.TotalCount,
218226
UnresolvedThreadCount: unresolvedThreadCount,
219227
ReviewCount: pr_.Reviews.TotalCount,
220-
PRReviewSignal: classifyPRReviewSignal(pr_.Body),
221228
ReviewDecision: pr_.ReviewDecision,
222229
}
230+
if pr_.Editor != nil {
231+
snap.PREditorLogin = pr_.Editor.Login
232+
snap.PREditorType = pr_.Editor.TypeName
233+
}
234+
snap.PRReviewSignal = classifyPRReviewSignal(pr_.Body, snap.PREditorType, snap.PREditorLogin)
223235
if pr_.UpdatedAt != "" {
224236
snap.PRUpdatedAt, _ = time.Parse(time.RFC3339, pr_.UpdatedAt)
225237
}
@@ -267,34 +279,35 @@ func CanFastSettleReview(snap *domain.ActivitySnapshot) bool {
267279
return true
268280
}
269281

270-
func classifyPRReviewSignal(body string) domain.PRReviewSignal {
282+
func classifyPRReviewSignal(body, editorType, editorLogin string) domain.PRReviewSignal {
283+
if !isCodexBotEditor(editorType, editorLogin) {
284+
return domain.PRReviewSignalNone
285+
}
271286
for _, line := range strings.Split(body, "\n") {
272287
line = strings.TrimSpace(line)
273288
line = strings.Trim(line, "-*#> \t")
274289
if line == "" {
275290
continue
276291
}
277292
lower := strings.ToLower(line)
278-
hasReviewContext := strings.Contains(lower, "codex") ||
279-
strings.Contains(lower, "review") ||
280-
strings.Contains(lower, "reviewed") ||
281-
strings.Contains(lower, "complete") ||
282-
strings.Contains(lower, "done")
283-
284-
if hasThumbsUpToken(line, lower) {
285-
if isStandaloneThumbsUp(line, lower) || hasReviewContext {
286-
return domain.PRReviewSignalApproved
287-
}
293+
if hasThumbsUpToken(line, lower) && isCompactReviewMarkerLine(line, lower, stripThumbsUpTokens) {
294+
return domain.PRReviewSignalApproved
288295
}
289-
if hasEyesToken(line, lower) {
290-
if isStandaloneEyes(line, lower) || hasReviewContext {
291-
return domain.PRReviewSignalReviewing
292-
}
296+
if hasEyesToken(line, lower) && isCompactReviewMarkerLine(line, lower, stripEyesTokens) {
297+
return domain.PRReviewSignalReviewing
293298
}
294299
}
295300
return domain.PRReviewSignalNone
296301
}
297302

303+
func isCodexBotEditor(typeName, login string) bool {
304+
normalizedLogin := strings.TrimSuffix(strings.ToLower(login), "[bot]")
305+
if normalizedLogin == "chatgpt-codex-connector" {
306+
return true
307+
}
308+
return typeName == "Bot" && strings.Contains(normalizedLogin, "codex")
309+
}
310+
298311
func hasEyesToken(line, lower string) bool {
299312
return strings.Contains(line, "👀") || strings.Contains(lower, ":eyes:")
300313
}
@@ -307,16 +320,33 @@ func hasThumbsUpToken(line, lower string) bool {
307320
strings.Contains(lower, "thumbs up")
308321
}
309322

310-
func isStandaloneEyes(line, lower string) bool {
311-
return line == "👀" || lower == ":eyes:"
323+
func isCompactReviewMarkerLine(line, lower string, stripToken func(string) string) bool {
324+
if len([]rune(line)) > 64 {
325+
return false
326+
}
327+
remaining := stripToken(lower)
328+
for _, token := range []string{
329+
"codex", "openai", "review", "reviewer", "status", "bot",
330+
"complete", "completed", "done", "approved", "running",
331+
"started", "starting", "in progress", "reviewing",
332+
} {
333+
remaining = strings.ReplaceAll(remaining, token, "")
334+
}
335+
remaining = strings.Trim(remaining, " \t:-_[](){}|/\\.,;")
336+
return remaining == ""
312337
}
313338

314-
func isStandaloneThumbsUp(line, lower string) bool {
315-
return line == "👍" ||
316-
lower == ":+1:" ||
317-
lower == ":thumbsup:" ||
318-
lower == ":thumbs_up:" ||
319-
lower == "thumbs up"
339+
func stripEyesTokens(lower string) string {
340+
lower = strings.ReplaceAll(lower, "👀", "")
341+
lower = strings.ReplaceAll(lower, ":eyes:", "")
342+
return lower
343+
}
344+
345+
func stripThumbsUpTokens(lower string) string {
346+
for _, token := range []string{"👍", ":+1:", ":thumbsup:", ":thumbs_up:", "thumbs up"} {
347+
lower = strings.ReplaceAll(lower, token, "")
348+
}
349+
return lower
320350
}
321351

322352
// Fingerprint computes a SHA-256 hash of the activity snapshot for change detection.
@@ -325,9 +355,10 @@ func isStandaloneThumbsUp(line, lower string) bool {
325355
func Fingerprint(snap *domain.ActivitySnapshot) string {
326356
h := sha256.New()
327357
fmt.Fprintf(h, "head:%s\n", snap.HeadSHA)
328-
fmt.Fprintf(h, "pr:%d:%d:%s:%s\n",
358+
fmt.Fprintf(h, "pr:%d:%d:%s:%s:%s\n",
329359
snap.PRUpdatedAt.UnixNano(),
330360
snap.PRLastEditedAt.UnixNano(),
361+
snap.PREditorType+"/"+snap.PREditorLogin,
331362
snap.PRReviewSignal,
332363
snap.ReviewDecision)
333364
fmt.Fprintf(h, "tc:%d utc:%d rc:%d\n", snap.ThreadCount, snap.UnresolvedThreadCount, snap.ReviewCount)

‎internal/github/activity_test.go‎

Lines changed: 45 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -184,40 +184,66 @@ func TestFingerprintChangesOnPRSignal(t *testing.T) {
184184

185185
func TestClassifyPRReviewSignal(t *testing.T) {
186186
tests := []struct {
187-
name string
188-
body string
189-
want domain.PRReviewSignal
187+
name string
188+
body string
189+
editorType string
190+
editorLogin string
191+
want domain.PRReviewSignal
190192
}{
191193
{
192-
name: "standalone eyes",
193-
body: "Implementation notes\n\n👀",
194-
want: domain.PRReviewSignalReviewing,
194+
name: "codex editor standalone eyes",
195+
body: "Implementation notes\n\n👀",
196+
editorType: "Bot",
197+
editorLogin: "chatgpt-codex-connector",
198+
want: domain.PRReviewSignalReviewing,
195199
},
196200
{
197-
name: "codex reviewing line",
198-
body: "- Codex review 👀",
199-
want: domain.PRReviewSignalReviewing,
201+
name: "codex editor reviewing line",
202+
body: "- Codex review 👀",
203+
editorType: "Bot",
204+
editorLogin: "chatgpt-codex-connector",
205+
want: domain.PRReviewSignalReviewing,
200206
},
201207
{
202-
name: "standalone thumbs up",
203-
body: "Ready\n\n👍",
204-
want: domain.PRReviewSignalApproved,
208+
name: "codex editor standalone thumbs up",
209+
body: "Ready\n\n👍",
210+
editorType: "Bot",
211+
editorLogin: "chatgpt-codex-connector",
212+
want: domain.PRReviewSignalApproved,
205213
},
206214
{
207-
name: "codex complete line",
208-
body: "Codex review complete :thumbsup:",
209-
want: domain.PRReviewSignalApproved,
215+
name: "codex editor complete line",
216+
body: "Codex review complete :thumbsup:",
217+
editorType: "Bot",
218+
editorLogin: "chatgpt-codex-connector",
219+
want: domain.PRReviewSignalApproved,
210220
},
211221
{
212-
name: "incidental thumbs up prose",
213-
body: "This feature gives users a thumbs up affordance.",
214-
want: domain.PRReviewSignalNone,
222+
name: "non codex editor standalone thumbs up",
223+
body: "👍",
224+
editorType: "Bot",
225+
editorLogin: "coderabbitai",
226+
want: domain.PRReviewSignalNone,
227+
},
228+
{
229+
name: "codex editor incidental thumbs up prose",
230+
body: "This feature gives users a thumbs up affordance.",
231+
editorType: "Bot",
232+
editorLogin: "chatgpt-codex-connector",
233+
want: domain.PRReviewSignalNone,
234+
},
235+
{
236+
name: "repo without codex has no signal",
237+
body: "👀",
238+
editorType: "User",
239+
editorLogin: "alice",
240+
want: domain.PRReviewSignalNone,
215241
},
216242
}
217243

218244
for _, tt := range tests {
219245
t.Run(tt.name, func(t *testing.T) {
220-
if got := classifyPRReviewSignal(tt.body); got != tt.want {
246+
if got := classifyPRReviewSignal(tt.body, tt.editorType, tt.editorLogin); got != tt.want {
221247
t.Fatalf("classifyPRReviewSignal() = %q, want %q", got, tt.want)
222248
}
223249
})

‎skill/SKILL.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ It waits for CI, performs bounded review monitoring, and returns everything in o
4646

4747
**Drop `--solo`** for org repos with required review policies.
4848
**Always include `--await-review`** when review comments may still arrive.
49-
`--await-review` understands Codex-style PR body review signals: eyes means the reviewer is
50-
still active, thumbs up can end the wait early, and the final status fetch still checks all
51-
threads, reviews, stale blockers, and CI.
49+
`--await-review` understands Codex-owned PR body review signals when Codex is enabled on the
50+
repo: eyes means the reviewer is still active, thumbs up can end the wait early, and the final
51+
status fetch still checks all threads, reviews, stale blockers, and CI. Without Codex, it keeps
52+
the conservative thread/review polling behavior.
5253
**Do not switch to bare `--watch`** after the first cycle if review comments still matter — `--watch` is CI-only and can miss follow-up bot comments.
5354
**Drop `--logs`** only on narrow re-checks where CI failure detail is definitely not needed.
5455

‎skill/references/command-reference.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -415,11 +415,12 @@ wasted timeout waiting for activity that already happened.
415415
after at least one activity change has been detected — prevents premature settlement while a
416416
bot is still working.
417417

418-
**PR review signals:** The activity probe also reads PR body status markers and PR reactions.
419-
Codex-style `eyes` markers keep review-await active. Codex-style `thumbs up` completion
420-
markers can settle the review wait early when there are no unresolved threads and GitHub does
421-
not report `CHANGES_REQUESTED`. The command still performs the normal final status fetch before
422-
reporting merge readiness.
418+
**PR review signals:** The activity probe also reads PR body status markers, the PR body editor,
419+
and PR reactions. Only Codex-owned markers are actionable: Codex `eyes` markers keep
420+
review-await active, and Codex `thumbs up` completion markers can settle the review wait early
421+
when there are no unresolved threads and GitHub does not report `CHANGES_REQUESTED`. Repos
422+
without Codex keep the conservative thread/review polling behavior. The command still performs
423+
the normal final status fetch before reporting merge readiness.
423424

424425
**Tail confirmation:** After the first quiet period, ghent performs bounded sparse confirmation
425426
probes before treating the review window as stable. If new activity appears during those probes,

0 commit comments

Comments
 (0)