diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index 2535f15..1b84f57 100644 --- a/.claude/memory/MEMORY.md +++ b/.claude/memory/MEMORY.md @@ -10,6 +10,7 @@ Keep this file under 200 lines — anything longer is content bloat, not memory. - [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/closed-pr-receives-review-after-close](learnings/2026-05-26-closed-pr-receives-review-after-close.md) — Closed PR can still get a REQUEST_CHANGES review seconds after dup-close; don't reopen, surface to sibling PR + Linear (PR #18 vs #20 ENG-25 race) +- [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` 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.