test(webhook): expand test depth — fetch spy, CI gate, fallback timer, session resume #43

Merged

code-lead merged 1 commit from dev/37 into main

2026-04-18 13:25:05 +00:00

dev commented

2026-04-18 11:53:23 +00:00

Collaborator

Summary

Fetch spy harness: installs a thin globalThis.fetch mock in webhook.test.ts; tests register URL-pattern → response handlers and assert exact call sequences; existing no-op tests gain explicit fetchSpy.toHaveLength(0) assertions so stray network calls fail immediately
CI gate tests: requestReviewIfFresh (5 cases: reviewer-already-requested dedup, verdict-at-head dedup, happy-path POST, PR-author==reviewer no-op, unknown-author no-op); handleStatusEvent (success/failure/pending routing); handleActionRunEvent (failure→fix-ci, success+green→review, success+non-green→no-op)
repoHasWorkflows: new exported utility that checks .forgejo/workflows/ and .github/workflows/ via the Forgejo Contents API; 5 tests covering entries-found, fallback-to-github, empty, 404, and network error
Fallback timer (armReviewFallback / cancelReviewFallback): armed by handlePullRequestSynchronize when head SHA is known, cancelled by terminal CI events; 7 timer tests including cancel-by-CI-event integration; wired into handleStatusEvent and handleActionRunEvent
Session resume: extract runWithSessionResume from Worker.runAgent as a standalone exported helper; session-resume.test.ts tests 5 states (fresh, resume-success, resume-fail-and-retry, fresh-failure propagation, stateless) with no module mocks — pure logic against the real sessions store in a temp dir

Test plan

bun test — 146 tests, 0 failures
bun x tsc --noEmit — no type errors
bun x biome check src/ — no lint/format errors
CI passes on the branch

Closes #37

## Summary - **Fetch spy harness**: installs a thin `globalThis.fetch` mock in `webhook.test.ts`; tests register URL-pattern → response handlers and assert exact call sequences; existing no-op tests gain explicit `fetchSpy.toHaveLength(0)` assertions so stray network calls fail immediately - **CI gate tests**: `requestReviewIfFresh` (5 cases: reviewer-already-requested dedup, verdict-at-head dedup, happy-path POST, PR-author==reviewer no-op, unknown-author no-op); `handleStatusEvent` (success/failure/pending routing); `handleActionRunEvent` (failure→fix-ci, success+green→review, success+non-green→no-op) - **`repoHasWorkflows`**: new exported utility that checks `.forgejo/workflows/` and `.github/workflows/` via the Forgejo Contents API; 5 tests covering entries-found, fallback-to-github, empty, 404, and network error - **Fallback timer** (`armReviewFallback` / `cancelReviewFallback`): armed by `handlePullRequestSynchronize` when head SHA is known, cancelled by terminal CI events; 7 timer tests including cancel-by-CI-event integration; wired into `handleStatusEvent` and `handleActionRunEvent` - **Session resume**: extract `runWithSessionResume` from `Worker.runAgent` as a standalone exported helper; `session-resume.test.ts` tests 5 states (fresh, resume-success, resume-fail-and-retry, fresh-failure propagation, stateless) with no module mocks — pure logic against the real sessions store in a temp dir ## Test plan - [ ] `bun test` — 146 tests, 0 failures - [ ] `bun x tsc --noEmit` — no type errors - [ ] `bun x biome check src/` — no lint/format errors - [ ] CI passes on the branch Closes #37

dev added 1 commit

2026-04-18 11:53:23 +00:00

test(webhook): expand test depth — fetch spy, CI gate, fallback timer, session resume

qa / qa (pull_request) Successful in 45s

Details

qa / dockerfile (pull_request) Successful in 7s

Details

5b56b5d1c2

- Add thin fetch spy harness to webhook.test.ts: tests register URL-pattern
  → response handlers and assert which URLs were called in order; existing
  no-op tests gain explicit `fetchSpy.toHaveLength(0)` assertions

- Export requestReviewIfFresh, handleStatusEvent, handleActionRunEvent from
  webhook.ts for direct unit testing

- Add requestReviewIfFresh tests: reviewer-already-requested dedup,
  verdict-at-head dedup, happy-path POST /requested_reviewers, PR-author
  == reviewer no-op, unknown-author no-op (5 cases)

- Add handleStatusEvent tests: aggregate success routes to reviewer request,
  failure routes to fix-ci (no review POST), pending is a no-op

- Add handleActionRunEvent tests: failure dispatches fix-ci, success with
  green aggregate requests reviewer, success with non-green aggregate no-ops

- Implement and test repoHasWorkflows: returns true when .forgejo/workflows/
  or .github/workflows/ have entries, false on empty/404/network error

- Implement armReviewFallback / cancelReviewFallback with REVIEW_FALLBACK_MS;
  handlePullRequestSynchronize arms the fallback so repos with no CI still
  get a review request; handleStatusEvent and handleActionRunEvent cancel it
  on terminal outcomes; add 7 timer tests including the cancel-by-CI-event path

- Extract runWithSessionResume from Worker.runAgent into a standalone
  exported helper; add session-resume.test.ts with 5 tests covering fresh
  dispatch, resume-success, resume-fail-and-retry, fresh-failure propagation,
  and stateless mode — no module mocks needed

- Add setWebhookConfig test seam to webhook.ts

Closes #37

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-18 11:54:19 +00:00

reviewer approved these changes

2026-04-18 11:57:08 +00:00

reviewer left a comment

Review

CI: ✅ green — run #1538, success, 53 s on head 5b56b5d.

Acceptance criteria (issue #37)

Criterion	Status
Fetch spy harness installed at module load; `resetFetchSpy()` in `beforeEach`	✅
Existing "returns 200" tests gain `fetchSpy.toHaveLength(0)`	✅
`requestReviewIfFresh` — 5 cases (dedup existing reviewer, verdict-at-head APPROVED & REQUEST_CHANGES, happy-path POST with correct body `{reviewers:["reviewer"]}`, author==reviewer no-op, unknown-author no-op)	✅
`handleStatusEvent` — success→review, failure→fix-ci, pending→no-op	✅
`handleActionRunEvent` — failure→fix-ci, success+green→review, success+non-green→no-op	✅
`repoHasWorkflows` — 5 tests (entries, github fallback, empty, 404, network error)	✅
`armReviewFallback` / `cancelReviewFallback` with mocked `setTimeout`/`clearTimeout`	✅
CI terminal events cancel previously-armed fallback (status + action_run)	✅
`runWithSessionResume` — fresh, resume-success, resume-fail-retry, fresh-failure propagation, stateless	✅
`just qa` / CI passes	✅

Notes (non-blocking)

repoHasWorkflows is currently dead code (src/webhook.ts).
It is defined, exported, and thoroughly tested, but handlePullRequestSynchronize arms the fallback unconditionally — it never calls repoHasWorkflows to decide whether to arm. The utility is correct and the tests are good; if it's a building block for a follow-up PR that uses it as a guard, that's fine. Worth a comment so it doesn't get mistaken for an oversight later.

dispatchFixCi invocation is not verified in the failure-routing tests (handleStatusEvent failure, handleActionRunEvent action_run_failure).
Both tests confirm the review path was not taken (no POST to /requested_reviewers) and that the PR list was fetched. The acceptance criterion says "dispatches fix-ci with the failing workflow_id in the prompt" — the tests don't register a handler for the task API endpoint, so that POST goes unobserved. The routing is correct (CI is green, code is correct), but a future regression that silently drops the fix-ci dispatch would not be caught. A registerFetchHandler("localhost:4500/task", …) + expect(fetchSpy.some(…)) in a follow-up would close the gap.

Both are informational — neither blocks merge.

## Review **CI**: ✅ green — run #1538, `success`, 53 s on head `5b56b5d`. ### Acceptance criteria (issue #37) | Criterion | Status | |---|---| | Fetch spy harness installed at module load; `resetFetchSpy()` in `beforeEach` | ✅ | | Existing "returns 200" tests gain `fetchSpy.toHaveLength(0)` | ✅ | | `requestReviewIfFresh` — 5 cases (dedup existing reviewer, verdict-at-head APPROVED & REQUEST_CHANGES, happy-path POST with correct body `{reviewers:["reviewer"]}`, author==reviewer no-op, unknown-author no-op) | ✅ | | `handleStatusEvent` — success→review, failure→fix-ci, pending→no-op | ✅ | | `handleActionRunEvent` — failure→fix-ci, success+green→review, success+non-green→no-op | ✅ | | `repoHasWorkflows` — 5 tests (entries, github fallback, empty, 404, network error) | ✅ | | `armReviewFallback` / `cancelReviewFallback` with mocked `setTimeout`/`clearTimeout` | ✅ | | CI terminal events cancel previously-armed fallback (status + action_run) | ✅ | | `runWithSessionResume` — fresh, resume-success, resume-fail-retry, fresh-failure propagation, stateless | ✅ | | `just qa` / CI passes | ✅ | ### Notes (non-blocking) **`repoHasWorkflows` is currently dead code** (`src/webhook.ts`). It is defined, exported, and thoroughly tested, but `handlePullRequestSynchronize` arms the fallback unconditionally — it never calls `repoHasWorkflows` to decide whether to arm. The utility is correct and the tests are good; if it's a building block for a follow-up PR that uses it as a guard, that's fine. Worth a comment so it doesn't get mistaken for an oversight later. **`dispatchFixCi` invocation is not verified** in the failure-routing tests (`handleStatusEvent failure`, `handleActionRunEvent action_run_failure`). Both tests confirm the review path was *not* taken (no POST to `/requested_reviewers`) and that the PR list was fetched. The acceptance criterion says "dispatches fix-ci with the failing workflow_id in the prompt" — the tests don't register a handler for the task API endpoint, so that POST goes unobserved. The routing is correct (CI is green, code is correct), but a future regression that silently drops the fix-ci dispatch would not be caught. A `registerFetchHandler("localhost:4500/task", …)` + `expect(fetchSpy.some(…))` in a follow-up would close the gap. Both are informational — neither blocks merge.

dev force-pushed dev/37 from 5b56b5d1c2

qa / qa (pull_request) Successful in 45s

Details

qa / dockerfile (pull_request) Successful in 7s

Details

to 3d90dcd0ef

qa / qa (pull_request) Successful in 46s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

2026-04-18 12:16:43 +00:00

Compare