From b76d753f611a3284638b97732fa9b9fb34db8159 Mon Sep 17 00:00:00 2001 From: Will Date: Tue, 26 May 2026 11:19:21 +0100 Subject: [PATCH 1/2] memory: note duplicate-session race when reviewer leaves multiple REQUEST_CHANGES comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewer agent left two near-identical AGENT_REVIEW: REQUEST_CHANGES comments on PR #20 (round 1) — likely its own retry/fanout — and the `/api/github-webhook` route fired a fresh manager session per comment. Two parallel sessions then each produced functionally identical fixes (matchAll + last-match, extracted module, ~12-case unit harness, playwright config widen). The first to push won; the second had to hard-reset and discard. No code damage, but wasted compute and time. Document the gotcha so future-me checks `git log --since="" origin/` before doing review-feedback work. --- .../2026-05-26-review-feedback-fanout.md | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .claude/memory/learnings/2026-05-26-review-feedback-fanout.md diff --git a/.claude/memory/learnings/2026-05-26-review-feedback-fanout.md b/.claude/memory/learnings/2026-05-26-review-feedback-fanout.md new file mode 100644 index 0000000..c2f6797 --- /dev/null +++ b/.claude/memory/learnings/2026-05-26-review-feedback-fanout.md @@ -0,0 +1,31 @@ +# Review-feedback fanout: re-check the head branch before doing work + +When the reviewer agent leaves *two* `AGENT_REVIEW: REQUEST_CHANGES` +comments seconds apart (e.g. its own webhook fanout, or a manual +re-review after edit), `/api/github-webhook` fires a fresh manager +session per comment. Each session is told "address the feedback on +the same branch, commit, push" — so two parallel sessions race to +produce the same fix. + +PR #20 hit this. Two sessions, both extracted `extractSessionId` into +a module, both swapped `match` for `matchAll` + last-match, both added +a ~12-case unit harness, both widened the playwright config. The +first to push won. The second discovered the race on `git push -- +rejected` and had to `git reset --hard origin/`. + +Mitigation at the agent level (until the webhook itself dedupes): +before branching/editing for review feedback, run +`git fetch origin ` then +`git log --since="" origin/` — if +a commit newer than the reviewer comment already addresses the +feedback, stand down with a Linear comment. + +Related: `conventions/check-open-pr-before-ticket-pickup.md` (analog +at ticket-pickup time, PR #15 vs #16) and +`learnings/2026-05-26-recheck-open-prs-at-pr-open.md` (analog at PR- +open time, PR #18 vs #20). Same race pattern, three webhook trigger +points. + +Cleaner fix lives in the webhook: dedupe by `(pr_number, head_sha, +comment.first_line)` and refuse to spawn a duplicate manager +session within N seconds. Tracked separately — out of scope here. From 3ea23b1190f4164443b9a7c378d5457c44b4d97b Mon Sep 17 00:00:00 2001 From: Will Date: Tue, 26 May 2026 11:19:42 +0100 Subject: [PATCH 2/2] memory: index entry for review-feedback fanout learning --- .claude/memory/MEMORY.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index f776d9f..c94b17d 100644 --- a/.claude/memory/MEMORY.md +++ b/.claude/memory/MEMORY.md @@ -9,6 +9,7 @@ Keep this file under 200 lines — anything longer is content bloat, not memory. - [learnings/sandbox-cant-clone-private-repo](learnings/2026-05-26-sandbox-cant-clone-private-repo.md) — Don't `git clone` from sandbox bash; the `github_repository` resource is auth'd, raw `git clone` is not - [learnings/github-mcp-strips-html-comments](learnings/2026-05-26-github-mcp-strips-html-comments.md) — `update_pull_request` silently strips `` from PR bodies; session-id marker can't be set by agent (ENG-25) - [learnings/recheck-open-prs-at-pr-open](learnings/2026-05-26-recheck-open-prs-at-pr-open.md) — session-start grep is insufficient; re-grep `list_pull_requests` right before `create_pull_request` to catch parallel-session races (PR #18 vs #20 ENG-25) +- [learnings/review-feedback-fanout](learnings/2026-05-26-review-feedback-fanout.md) — multiple REQUEST_CHANGES comments fire multiple manager sessions; fetch + check head-branch commits newer than the reviewer comment before doing work (PR #20) ## Decisions - [decisions/mcp-for-small-writes-checkout-for-big](decisions/2026-05-26-mcp-for-small-writes-checkout-for-big.md) — Single-file writes go through GitHub MCP; multi-file or test-needing changes use the mounted checkout + `git push`