Skip to content

Evidence for #74: review run 8 on PR #78 started 45 seconds before merge, findings discarded #79

Description

@fullsend-ai-retro

What happened

On PR #78, review run 8 was dispatched at 20:53:45 UTC on the final commit (4467568). The PR was self-merged by ralphbean at 20:54:30 UTC — 45 seconds later. The review agent had no time to complete. Any results from this run would have been discarded by post-review.sh's merged-PR early exit path. This is the third documented instance of this pattern in the agents repo (after PR #73 and PR #59 documented in issue #74).

Notably, this PR received 0 completed fullsend reviews despite 8 dispatch attempts. The only completed automated review was from Qodo, which identified a path-filter gap (later fixed by the author) and a security concern about overbroad allowed_remote_resources (not addressed before merge).

What could go better

The post-merge review delivery fix proposed in #74 would partially address this — if the review had completed after merge, its findings would have been posted as an informational comment rather than silently discarded. However, in this specific case the review had only 45 seconds and likely did not complete meaningful analysis.

The more impactful fix is the debounce mechanism (#1014 upstream), which would have prevented the first 6-7 dispatches and allowed a review to start and complete on an earlier commit. The two fixes are complementary: debounce reduces wasted dispatches during iteration, while post-merge delivery salvages reviews that complete after merge.

Confidence: High that this is the same pattern as #74. Medium confidence that this specific instance would have benefited from #74's fix (since the review likely did not complete in 45 seconds).

Proposed change

This is additional evidence for #74 (post-review should deliver findings on merged PRs). The new data point: PR #78 review run 8 started at 20:53:45 UTC, PR merged at 20:54:30 UTC (45 seconds later), marking the third instance of this pattern in the agents repo. This case also demonstrates the interaction between #74 and fullsend#1014 — if dispatch had been debounced, the review would have started sooner on a settled SHA and likely completed before merge, making #74's fix unnecessary for this PR. No new issue needed; this comment should be appended to #74 as supporting evidence.

Validation criteria

After implementing #74: if a review agent completes after a PR is merged, findings appear as an informational comment (not a blocking review) on the merged PR. For this specific pattern (review started <1 minute before merge), verify the post-script correctly handles the race condition where the PR state transitions from open to merged during agent execution.


Generated by retro agent from #78

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    duplicateThis issue or pull request already existsready-for-triageTriggers triage agent dispatch

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions