fix(skills): prevent /review from spamming @greptileai when already approved

[carlos-alm]

Summary

• Fixes the /review skill blindly re-triggering @greptileai even when Greptile already approved (thumbsup + no inline comments), as seen on PR refactor: address SLOC warnings in domain and features layers #621
• Replaces the single emoji-on-reply check with a proper decision matrix that checks the trigger comment's thumbsup reaction, inline comment count, and elapsed time
• Adds a 15-minute wait rule: if Greptile hasn't responded yet, wait instead of posting duplicate triggers

Test plan

• Run /review on a PR where Greptile already gave thumbsup + no comments — verify it skips re-triggering
• Run /review on a PR with outstanding Greptile inline comments — verify it addresses them and re-triggers
• Run /review on a PR where @greptileai was just posted (<15 min ago) — verify it waits instead of posting again

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Analysis

Reviewing PR #628: fix(skills): prevent /review from spamming @greptileai when already approved

Todo List

• Read modified skill file to understand changes
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity
• Phase 4: Code Quality
• Phase 5: Backlog Compliance Assessment
• Provide final rating and recommendation

---

Root Cause Analysis

Problem Being Solved: The /review skill was blindly re-triggering @greptileai even when Greptile had already approved a PR (indicated by thumbsup reaction + no new inline comments), leading to spam and unnecessary reviewer pings.

Why This Approach: Replaces a simplistic "check reactions on your last reply" method with a sophisticated decision matrix that:

• Finds the most recent @greptileai trigger comment (from anyone)
• Checks for thumbsup reactions from greptile-apps[bot]
• Counts inline comments posted after the trigger
• Implements a 15-minute wait rule to prevent duplicate triggers

Risk Assessment: LOW RISK. This is a pure logic improvement to an automation workflow with no impact on core functionality.

Backlog Compliance

• Zero-dep: ✓ — No new dependencies, uses existing gh CLI and jq
• Foundation-aligned: ✓ — Improves automation efficiency without violating core principles
• Problem-fit: 2 — Nice workflow improvement, but doesn't address the core "lost AI" problem
• Breaking: No — Pure internal skill improvement with no API changes
• Tier: 1

Critical Concerns

None identified. The implementation is well-structured and addresses the root cause properly.

Technical Analysis

✅ Strengths:

1. Sophisticated decision logic: Implements proper state tracking with multiple conditions rather than naive emoji checking
2. Time-based protection: 15-minute wait rule prevents race conditions and duplicate triggers
3. Correct API usage: Uses proper GitHub API endpoints (/comments, /reactions) with appropriate filtering
4. Clear documentation: Decision matrix is well-explained with explicit logic branches
5. Preserves existing behavior: Only changes the Greptile re-trigger logic, maintains all other functionality

✅ Code Quality:

• Bash scripting: Proper use of jq for JSON parsing and GitHub CLI for API calls
• Error handling: Appropriate use of pagination (--paginate) for complete data retrieval
• Variable naming: Clear variable names (trigger_comment, trigger_id, trigger_time)
• Logic flow: Well-structured conditional branches with clear exit conditions

✅ No Config/Test Integrity Issues:

• No configuration files modified
• No test expectations lowered
• No linting rules disabled
• This is pure workflow logic enhancement

🔍 Minor Observations:

1. The old logic was checking reactions on "your last reply" vs. new logic checks reactions on "last @greptileai trigger" — this is actually more correct since triggers can come from anyone
2. New logic is more complex but necessarily so to handle edge cases properly
3. The 15-minute wait rule is a reasonable heuristic to balance responsiveness vs. spam prevention

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: This is a well-implemented fix for a real workflow issue. The approach is sound, the logic is comprehensive, and there are no technical concerns. The only reason it's not 5 stars is that it's an incremental improvement rather than addressing core codegraph functionality, but within its scope it's excellent work.

The decision matrix properly handles all the edge cases mentioned in the PR description and should eliminate the spam behavior while maintaining proper reviewer workflow.

---

| Branch: fix/review-greptile-retrigger

[greptile-apps]

Greptile Summary

This PR hardens the /review skill's Greptile re-triggering logic, replacing a fragile single emoji-reaction check with a well-structured decision matrix that accounts for thumbsup reactions, new inline comments, and elapsed time since the last trigger.\n\nKey changes:\n- Added a trigger_count guard that skips the approval check entirely when no prior human @greptileai comment exists, preventing null-ID API calls.\n- Filtered select(.user.login != \"greptile-apps[bot]\") from both the count query and the | last trigger-lookup, so Greptile's own review responses are no longer mistaken for human trigger comments.\n- Introduced a portable python3-based elapsed-minutes calculation to make the >15 min decision branches concrete.\n- Extended the decision matrix to all 6 meaningful states, including the two previously unhandled cases.\n- All four prior review-thread concerns are addressed in this revision.\n\nRemaining concern: When trigger_count -eq 0 the script falls through to Check 2, which says "if no code changes in response to Greptile comments, skip re-triggering." For a brand-new PR that has never had @greptileai posted, Check 2's condition is trivially true, so an agent applying the instructions strictly could skip Greptile's very first trigger.

Confidence Score: 4/5

Safe to merge with one low-risk edge case to address: Check 2 may suppress the first-ever @greptileai trigger on a fresh PR.

All four prior review-thread concerns are correctly resolved (null-trigger guard, bot-comment exclusion, missing decision branches, elapsed-time tooling). The bash snippets and jq filters are syntactically correct and the decision matrix is now exhaustive. One remaining P2 ambiguity exists — the first-ever trigger case routing through Check 2 — but it only affects PRs where @greptileai was never previously posted, and the overall logic is sound. Rewarding convergence: prior concerns fully addressed.

.claude/skills/review/SKILL.md lines 213–247 (first-run trigger_count=0 path and Check 2 interaction)

Important Files Changed

Filename	Overview
.claude/skills/review/SKILL.md	Replaces single emoji-reaction check with a 6-branch decision matrix including elapsed-time computation, null-trigger guard, and Greptile-bot comment exclusion; all four prior review concerns addressed; one minor logic ambiguity remains in the Check 2 path for first-ever triggers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Step 2g: Re-trigger Greptile?] --> B{trigger_count == 0?}
    B -- Yes --> C[No prior trigger\nProceed to Check 2]
    B -- No --> D[Get last trigger comment\nid + created_at]
    D --> E[Fetch thumbsup_count\nfetch inline_count\ncompute elapsed_minutes]
    E --> F{thumbsup exists?}
    F -- Yes --> G{new inline comments?}
    G -- Yes --> H[Address inline comments\nthen re-trigger]
    G -- No --> I{elapsed > 15 min?}
    I -- Yes --> J[Greptile approved\nSkip re-triggering]
    I -- No --> K[Still processing\nWait, then re-check]
    F -- No --> L{elapsed > 15 min?}
    L -- No --> M[Not responded yet\nWait, then re-check]
    L -- Yes --> N{new inline comments?}
    N -- Yes --> O[Address inline comments\nthen re-trigger]
    N -- No --> P[Never responded\nRe-trigger once]
    C --> Q{Check 2: any code changes\nin response to Greptile?}
    H --> Q
    O --> Q
    P --> Q
    Q -- No --> R[Skip re-triggering]
    Q -- Yes --> S[Post @greptileai]

Loading

_{Reviews (2): Last reviewed commit: "fix(skills): address Greptile review fee..." | Re-trigger Greptile}

[greptile-apps]

Decision matrix has two unhandled cases

The four listed cases leave gaps that will leave the agent with no instruction:

1. No thumbsup AND >15 min AND no new inline comments — Greptile never responded at all (no reaction, no inline comments) after the full wait. The matrix says nothing about this state. The agent will likely stall or make an arbitrary choice. The correct behavior here is probably to re-trigger (Greptile may have missed the notification).

2. Thumbsup exists AND new inline comments exist (regardless of time) — Greptile approved but also left inline comments. This is a contradictory-but-real state (e.g., Greptile leaves nit comments but still thumbsups). The matrix doesn't say whether to treat the thumbsup or the inline comments as the authoritative signal.

Suggested additions to the decision logic:

- No thumbsup AND >15 min AND no new inline comments → **Greptile never responded. Re-trigger once** (it may have missed the notification).
- Thumbsup exists AND new inline comments exist → **Treat inline comments as outstanding concerns. Address them (step 2e), then re-trigger.**

[carlos-alm]

Fixed. Added both missing branches to the decision matrix:

• Thumbsup + inline comments → Treat inline comments as outstanding concerns. Address them, then re-trigger. (Greptile sometimes thumbsups but still leaves nit comments.)
• No thumbsup + >15 min + no inline comments → Greptile never responded. Re-trigger once (it may have missed the notification).

[greptile-apps]

No guard when no prior @greptileai trigger exists

[...] | last on an empty array returns null in jq. If a PR has never had an @greptileai comment (first run), trigger_id and trigger_time are both the string "null", causing the subsequent reactions API call to hit /issues/comments/null/reactions (a 404) and the inline-comments filter to compare timestamps against the literal string "null".

The agent should detect the empty case upfront and skip the matrix entirely (jumping straight to check 2 / re-trigger):

trigger_count=$(gh api repos/optave/codegraph/issues/<number>/comments --paginate \
  --jq '[.[] | select(.body | test("@greptileai"))] | length')
if [ "$trigger_count" -eq 0 ]; then
  # No prior trigger — skip approval check, proceed to check 2
  ...
fi

[carlos-alm]

Fixed. Added a \ check upfront — if no prior \ trigger exists (excluding Greptile's own responses), the script skips the approval check entirely and proceeds to check 2 (whether we addressed any Greptile feedback). This avoids the null ID/timestamp issue.

[greptile-apps]

test("@greptileai") will match Greptile's own response comments

Greptile's review responses frequently contain the phrase @greptileai in their body (e.g. "I reviewed this as requested via @greptileai"). The select(.body | test("@greptileai")) filter picks up those Greptile-authored comments too, so | last may resolve to a Greptile response rather than the human trigger comment. This would cause the reactions and inline-comment checks to be run against the wrong comment ID/timestamp.

The filter should exclude Greptile's own comments:

trigger_comment=$(gh api repos/optave/codegraph/issues/<number>/comments --paginate \
  --jq '[.[] | select(.body | test("@greptileai")) | select(.user.login != "greptile-apps[bot]")] | last | {id: .id, created_at: .created_at}')

[carlos-alm]

Fixed. The jq filter now excludes Greptile's own comments with select(.user.login != "greptile-apps[bot]") in both the count query and the last-trigger query, so | last will only resolve to an actual human trigger comment.

[greptile-apps]

No tooling provided for the elapsed-time check

The decision branches on >15 min vs <15 min since the trigger, but the bash block gives the agent no command to compute elapsed time. The agent receives $trigger_time (an ISO 8601 string) and must infer "now" on its own — which is reliable only if the agent's clock/date context is accurate.

A concrete command removes ambiguity:

# Elapsed time in seconds since the trigger
elapsed_seconds=$(( $(date -u +%s) - $(date -u -d "$trigger_time" +%s 2>/dev/null || date -u -j -f "%Y-%m-%dT%H:%M:%SZ" "$trigger_time" +%s) ))
elapsed_minutes=$(( elapsed_seconds / 60 ))
echo "Minutes since trigger: $elapsed_minutes"

(The date -d form works on Linux; the -j -f form works on macOS — providing both or using python3 -c makes it portable.)

[carlos-alm]

Fixed. Added concrete elapsed-time computation using python3 for ISO 8601 parsing (portable across platforms) and date -u +%s for current epoch. The result is stored in $elapsed_minutes which the decision logic references directly.

[carlos-alm]

@greptileai