Tests: end-to-end test for the force-merge-on-MAX_ROUNDS path #140

New issue

Closed

opened 2026-04-20 13:01:28 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-20 13:01:28 +00:00

Collaborator

User story

As a developer, I want an integration test that drives handleChangesRequested → guardAuthorDispatch → dispatchMerge({ force: true }) end-to-end so that regressions in the MAX_ROUNDS terminator (#137) are caught at commit time instead of in production.

Context

PR #137 added the auto-force-merge behavior: on the reviewer's 3rd REQUEST_CHANGES, the address-review dispatch is skipped and a force-merge is dispatched to boss instead. Coverage today:

src/review-loop.test.ts — unit test for guardAuthorDispatch. Asserts forceMerge: true, reason contains "force-merging", comment body. Good.
Missing: an integration test that runs handleChangesRequested(repo, pr) with a reviewer mock returning MAX_ROUNDS REQUEST_CHANGES, and asserts:
- dispatchMerge was called with { force: true }
- the task text sent to the enqueued boss worker contains "⚠️ FORCE MERGE — review-loop terminator" (the prefix string defined in webhook-ci.ts)
- the boss worker was actually enqueued (queue depth went from 0 to 1)

Without this test, someone refactoring the handler could drop the cap.forceMerge branch and only the operator would notice (via a stuck PR on the next runaway review loop).

Acceptance criteria

Test file

New suite in src/webhook-handlers.test.ts (or a new force-merge.integration.test.ts if cleaner) titled handleChangesRequested MAX_ROUNDS force-merge path.
Seeds Forgejo fetch-mock with:
- GET /pulls/:n — mergeable: true
- GET /pulls/:n/reviews — MAX_ROUNDS REQUEST_CHANGES reviews by reviewer
- POST /issues/:n/comments — accept (for the capped-auto-merge comment)
Registers a stub boss worker that records enqueued tasks.
Calls handleChangesRequested(repo, pr); asserts:
- return value is the boss task id (not null)
- stub worker received exactly one task
- the task's task string starts with the FORCE MERGE prefix
- no address-review task was enqueued to the author agent

Pre-force-merge path test (negative)

Below MAX_ROUNDS: address-review is dispatched, no boss task enqueued, no force-merge comment posted. (This may already be covered by existing tests — extend if needed.)

Out of scope

Testing the merge skill's own "skip APPROVED check when force" logic — that's in the skill text, not in TS.
Testing the rebase-before-cap behavior (already covered by handleChangesRequested's unit tests).

References

Force-merge wiring: src/review-loop.ts::guardAuthorDispatch, src/webhook-ci.ts::dispatchMerge, src/webhook-handlers.ts::handleChangesRequested.
Unit test covering the guard: src/review-loop.test.ts::guardAuthorDispatch.
Force-merge prefix string: src/webhook-ci.ts — "⚠️ FORCE MERGE — review-loop terminator".

Dependencies

Blocked by: nothing (#137 merged).
Blocks: nothing.
Branch off: main.

## User story As a **developer**, I want an integration test that drives `handleChangesRequested → guardAuthorDispatch → dispatchMerge({ force: true })` end-to-end so that regressions in the MAX_ROUNDS terminator (#137) are caught at commit time instead of in production. ## Context PR #137 added the auto-force-merge behavior: on the reviewer's 3rd REQUEST_CHANGES, the address-review dispatch is skipped and a force-merge is dispatched to boss instead. Coverage today: - `src/review-loop.test.ts` — unit test for `guardAuthorDispatch`. Asserts `forceMerge: true`, reason contains "force-merging", comment body. Good. - **Missing**: an integration test that runs `handleChangesRequested(repo, pr)` with a reviewer mock returning `MAX_ROUNDS` REQUEST_CHANGES, and asserts: - `dispatchMerge` was called with `{ force: true }` - the task text sent to the enqueued boss worker contains `"⚠️ FORCE MERGE — review-loop terminator"` (the prefix string defined in `webhook-ci.ts`) - the boss worker was actually enqueued (queue depth went from 0 to 1) Without this test, someone refactoring the handler could drop the `cap.forceMerge` branch and only the operator would notice (via a stuck PR on the next runaway review loop). ## Acceptance criteria ### Test file - [ ] New suite in `src/webhook-handlers.test.ts` (or a new `force-merge.integration.test.ts` if cleaner) titled `handleChangesRequested MAX_ROUNDS force-merge path`. - [ ] Seeds Forgejo fetch-mock with: - `GET /pulls/:n` — mergeable: true - `GET /pulls/:n/reviews` — `MAX_ROUNDS` REQUEST_CHANGES reviews by reviewer - `POST /issues/:n/comments` — accept (for the capped-auto-merge comment) - [ ] Registers a stub boss worker that records enqueued tasks. - [ ] Calls `handleChangesRequested(repo, pr)`; asserts: - return value is the boss task id (not null) - stub worker received exactly one task - the task's `task` string starts with the FORCE MERGE prefix - no address-review task was enqueued to the author agent ### Pre-force-merge path test (negative) - [ ] Below MAX_ROUNDS: address-review is dispatched, no boss task enqueued, no force-merge comment posted. (This may already be covered by existing tests — extend if needed.) ## Out of scope - Testing the merge skill's own "skip APPROVED check when force" logic — that's in the skill text, not in TS. - Testing the rebase-before-cap behavior (already covered by `handleChangesRequested`'s unit tests). ## References - Force-merge wiring: `src/review-loop.ts::guardAuthorDispatch`, `src/webhook-ci.ts::dispatchMerge`, `src/webhook-handlers.ts::handleChangesRequested`. - Unit test covering the guard: `src/review-loop.test.ts::guardAuthorDispatch`. - Force-merge prefix string: `src/webhook-ci.ts` — "⚠️ FORCE MERGE — review-loop terminator". ## Dependencies - **Blocked by:** nothing (#137 merged). - **Blocks:** nothing. - **Branch off:** `main`.

claude-desktop added the

area:webhook

type:chore

labels

2026-04-20 13:02:37 +00:00