Skip to content

fix(pr-monitor): decide comment new-ness by id and timestamp, not body text #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aliasunder merged 3 commits into from
Sep 6, 2026
Merged

fix(pr-monitor): decide comment new-ness by id and timestamp, not body text #13

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c139628
fix(pr-monitor): decide comment new-ness by id and timestamp, not bod…
aliasunder Sep 6, 2026
1944091
fix(pr-monitor): fetch the inline-comments endpoint in Step 2d
aliasunder Sep 6, 2026
5fd6e9f
fix(pr-monitor): count inline comments in the empty-state report, use…
aliasunder Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions plugins/ship-check/skills/pr-monitor/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Fetch both surfaces with pagination to ensure no comments are missed:
```
gh api repos/OWNER/REPO/pulls/NUMBER/reviews --paginate --jq '.[] | {id, user: .user.login, submitted_at, body}'
gh api repos/OWNER/REPO/issues/NUMBER/comments --paginate --jq '.[] | {id, user: .user.login, created_at, body}'
gh api repos/OWNER/REPO/pulls/NUMBER/comments --paginate --jq '.[] | {id, user: .user.login, created_at, path, body}'
```

**Classify every comment** — not just bot-authored ones. Use the same
Expand All @@ -117,10 +118,11 @@ one-liner before evaluating:
```
Issue comment #<id> (<author>, <classification>): <first-line summary>
Review body #<id> (<author>, <classification>): <first-line summary>
Inline comment #<id> (<author>, <classification>): <path> — <first-line summary>
```
This listing is mandatory — it proves you fetched and read every comment.
If the listing is empty, state "0 issue comments, 0 review bodies with
Comment thread
aliasunder marked this conversation as resolved.
findings" explicitly.
findings, 0 inline comments" explicitly.

**Review bodies are NOT summaries — parse them for embedded findings.**
Bot review bodies routinely contain findings that were NOT posted as inline
Expand Down Expand Up @@ -159,12 +161,30 @@ against the review/comment IDs recorded on prior passes and against your
own footer-marked replies. **Record the IDs of items you handle** so
follow-up passes don't re-litigate them.

**New-ness is decided by timestamps and ids, never by body text.** After
any re-review event, list comments from BOTH endpoints (inline review
comments and issue comments) with `id` and `created_at`, and treat a
comment as new only if its `created_at` postdates the run that supposedly
produced it. An identical body with a familiar tracking marker is evidence
you re-read the SAME comment — a "duplicate posting" verdict requires two
distinct comment ids. The known failure this prevents: an
issue-comments-only query re-read a round-1 comment as a "duplicate" on
two consecutive re-reviews while the bot's actual new findings — posted as
inline review comments — went unseen and unhandled.
Comment thread
aliasunder marked this conversation as resolved.

### 2e. Merge readiness
Summarize blockers: failing CI, missing approvals, unresolved threads,
unanswered non-thread bot findings, conflicts.

## Step 3: Handle findings

> **Every reply this step posts carries the pipeline attribution footer**
> (`\n\n---\n*🔍 ship-check · pr-monitor · <model-id>*`) — never the
> generic direct-post "Claude Code" line. If you were posting outside a
> pipeline earlier in the session, the footer switches the moment this
> skill is running — re-read this line before the first reply of every
> monitoring cycle.

### Bot threads (qodo, CodeRabbit, etc.)

For each unresolved bot thread, do ALL of these in order:
Expand Down Expand Up @@ -307,8 +327,10 @@ Do NOT go to Step 5 without completing at least one follow-up pass after the las
```

3. On wake: **re-run Step 2** (all five checks). Compare the unresolved thread count
to what it was before pushing, and compare 2d's review/comment IDs against the
ones you've already handled.
to what it was before pushing, and compare 2d's review/comment IDs and
per-surface timestamps (`submitted_at` for review bodies, `created_at`
for issue and inline comments) against the ones you've already handled —
new-ness is decided by id and timestamp, never by body text (see 2d).

4. **New unresolved threads or new non-thread bot findings exist** -- go to Step 3
(reply, fix, resolve, push). If Step 3 pushes more code, return here and repeat
Expand Down