test(github-personas): integration coverage for self-directed review requests

[aaight]

Summary

Adds integration test coverage for the self-directed review-request path — where a human contributor whose GitHub account also holds the shared GITHUB_TOKEN_REVIEWER re-requests their own review (sender === requested_reviewer). This locks in the shared-reviewer-token behavior (MNG-1746) end-to-end against a real Postgres + Redis-backed project so it cannot silently regress.

Test-only change — no production code is modified.

Issue: https://linear.app/issue/MNG-1747
(Full URL: https://linear.app/mongrel/issue/MNG-1747/as-a-developer-i-want-integration-test-coverage-for-self-directed)

What changed

tests/integration/github-personas.test.ts:

1. New test case in the ReviewRequestedTrigger describe block:
   triggers review for a self-directed request (sender === requested_reviewer, shared reviewer token).
   It seeds the review agent + an enabled scm:review-requested trigger config, builds a payload where sender === requested_reviewer === TEST_PERSONAS.reviewer, and asserts result?.agentType === 'review'.

2. makeReviewRequestedPayload gained an explicit sender override — a new optional third parameter senderLogin that defaults to prAuthor, so all three existing call sites keep their current behavior unchanged. Passing it explicitly lets the payload model a self-directed request (sender === requested_reviewer).

Why this is a meaningful regression guard

The existing positive-dispatch test (triggers review when enabled via DB and persona is requested) uses a non-persona sender (external-user), so it never exercises the isCascadeBot(senderLogin) === true branch. This new case does: the sender is a CASCADE persona, so the naive loop-prevention guard would skip it. Only the self-directed exemption (sender === requested_reviewer) lets it fall through to dispatch. If MNG-1746's relaxed guard were reverted, handle() would return a loop-prevention skip and result?.agentType would no longer be 'review', failing this assertion.

Testing

TEST_DATABASE_URL=... REDIS_URL=... npx vitest run --project integration tests/integration/github-personas.test.ts

• ✅ Targeted integration run: 27 tests passed (was 26 — the new case is the +1; the 3 pre-existing ReviewRequestedTrigger cases still pass with the modified helper)
• ✅ npx biome check tests/integration/github-personas.test.ts — clean
• ✅ npm run typecheck — clean

Out of scope

• The production handler change (delivered in the core fix, MNG-1746)
• Unit tests and docs
• Any change to shared integration harness setup beyond the new test case + the payload-helper sender override

Dependencies

Depends on MNG-1746 (self-directed review requests dispatch the review agent) — already merged; the relaxed guard is present in src/triggers/github/review-requested.ts, which is why the agentType === 'review' assertion holds.

🤖 Generated with Claude Code

🕵️ claude-code · claude-opus-4-8 · run details

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

APPROVE — a clean, well-scoped test-only change that adds a genuine regression guard for the self-directed review-request path (MNG-1747), fully satisfying the work item's acceptance criteria.

Verification

• Backward compatibility: makeReviewRequestedPayload gained senderLogin: string = prAuthor. The default reproduces the prior sender: { login: prAuthor } behavior, so all three pre-existing 2-arg call sites are unchanged.
• Meaningful guard (traced through src/triggers/github/review-requested.ts): with sender === requested_reviewer === TEST_PERSONAS.reviewer, isSelfDirectedReviewRequest is true, so the loop-prevention guard (isCascadeBot(sender) && !isSelfDirectedReviewRequest) does not skip and the handler dispatches review. If MNG-1746's && !isSelfDirectedReviewRequest were reverted, handle() would return a skip (agentType: null) and the expect(result?.agentType).toBe('review') assertion would fail — exactly the intended regression net.
• No cross-test dedup interference: both dispatch tests share the same Redis dedup key (owner/repo:42:abc123) and truncateAll() only clears Postgres, but handle() calls releaseReviewDispatch() before claimReviewDispatch(), making the tests order-independent.
• AC + CI: all five MNG-1747 acceptance criteria met; 7/7 checks green.

No blocking, should-fix, or noteworthy nitpick findings.

🕵️ claude-code · claude-opus-4-8 · run details