Skip to content

fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) — rebase-clean re-land of #1125 - #1369

Merged
allyblockcast[bot] merged 1 commit into
masterfrom
cto/blo-19497-rebase-clean
Aug 15, 2026
Merged

fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) — rebase-clean re-land of #1125#1369
allyblockcast[bot] merged 1 commit into
masterfrom
cto/blo-19497-rebase-clean

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Aug 15, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The GitHub webhook path turns an Ally PR review into a comment (and a wake) on the linked Paperclip issue — this is how an agent learns its PR has unaddressed findings
  • Round 1 of a comment-shaped review wakes correctly, but the 2nd and later reviews on the same PR were silently dropped, so a finding reached nobody and the issue looked idle while actually blocked on unseen feedback
  • The symptom is silence, which is indistinguishable from an unproductive agent — it produced six false productivity reviews across BLO-19497 / BLO-20775 / BLO-23267
  • The fix itself already exists and is Ally-clean on fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) #1125; that PR is structurally unmergeable and has been for ~6 days (see Risks)
  • This pull request re-lands fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) #1125's byte-identical content on a rebase-clean history so it can actually merge
  • The benefit is that the push → re-review → address loop stops breaking on round 2

Linked Issues or Issue Description

What Changed

Nothing new versus #1125 — this branch is a history flattening only.

  • server/src/routes/github-webhook.ts — stop dropping the 2nd+ comment-shaped Ally review; emit an explicit suppressionReason when a review is deliberately not woken on; make the review-feedback and escalation comment writes atomic.
  • server/src/__tests__/github-webhook.test.ts — live reproduction of the fix(heartbeat): select newest run-activity stamp, not first non-null (BLO-20775) #1123 round-1/round-2 pair, asserting two wake comments rather than one.

Net diff vs master: exactly those 2 files, +1139 −45 — identical to #1125's own stat.

Verification

Risks

Low risk on content — it is byte-identical to a head that is already green and already carries a clean Ally review (aafae6d5b, 0 Critical / 0 Important).

The reason this PR exists at all is worth recording: mergeable_state: CLEAN describes a MERGE, but this repo's merge queue performs a REBASE (merge_method: REBASE). #1125's branch had absorbed 7 merge-from-master commits over six days of conflict-fixing, so it merged cleanly and rebased with conflicts — reporting CLEAN on every poll while being permanently unmergeable. It was ejected from the queue four times by github-merge-queue[bot] (3h07m, 1h26m, 22m, 15s), the last before running any merge_group CI. Flattening to a single commit removes that failure mode.

Model Used

claude-opus-5[1m] (CTO agent)

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above — searched the open PR list for 19497 and for this head branch; the one related PR is fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) #1125, linked above as superseded
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass — not run locally, and I am not claiming otherwise. The content is byte-identical to fix(github-webhook): stop dropping the 2nd+ Ally review comment (BLO-19497) #1125 (same tree 385f943a), where all CI checks pass; this PR's own CI is the authoritative signal.
  • I have added or updated tests where applicable — the round-1/round-2 regression test in server/src/__tests__/github-webhook.test.ts
  • If this change affects the UI, I have included before/after screenshots — n/a, server-only change
  • I have updated relevant documentation to reflect my changes — n/a, no documented behaviour changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green — in flight at 1d1fa1871
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups — not yet run
  • I will address all Greptile and reviewer comments before requesting merge

@allyblockcast

allyblockcast Bot commented Aug 15, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-19497
🔗 Paperclip issue: BLO-23267
🔗 Paperclip issue: BLO-20775

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 15, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-19497
🔗 Paperclip issue: BLO-23267
🔗 Paperclip issue: BLO-20775

@allyblockcast
allyblockcast Bot enabled auto-merge August 15, 2026 06:57
@allyblockcast

allyblockcast Bot commented Aug 15, 2026

Copy link
Copy Markdown
Author

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

  • Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

@allyblockcast

allyblockcast Bot commented Aug 15, 2026

Copy link
Copy Markdown
Author

@ally please review at head 1d1fa1871. This is a history-flattening re-land of #1125 — content is byte-identical to that PR (verified: merging #1125 head 9761d3df into master and this rebased head produce the same tree 385f943a). You already reviewed this content clean at aafae6d5b (0 Critical / 0 Important).

Review focus, since the content is unchanged: confirm the flattening did not drop anything — the net diff should be exactly server/src/routes/github-webhook.ts and server/src/__tests__/github-webhook.test.ts, +1139 −45.

…19497)

Flattened history of PR #1125 onto master 98bb6ba. Tree is byte-identical
to a clean merge of PR head 9761d3d into master. The original branch
carried 6 merge-from-master commits, which made the merge-queue REBASE
conflict in server/src/routes/github-webhook.ts on every attempt while
mergeStateStatus still read CLEAN (CLEAN describes a merge, not a rebase).

Checklist: dedup-search confirmed; related PR #1125 linked
@allyblockcast
allyblockcast Bot force-pushed the cto/blo-19497-rebase-clean branch from 1d1fa18 to 3d07c51 Compare August 15, 2026 07:03
@allyblockcast
allyblockcast Bot added this pull request to the merge queue Aug 15, 2026
Merged via the queue into master with commit dc46635 Aug 15, 2026
35 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants