test(review-residue): unit tests for the two runner paths — the PR-body SEQUANT_MUTATION merge and resolveReadFirstPR's state-record fallback with its open-PR guard

Open
  • Is something wrong?
#1,588 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
55/100
Issue type
Feature
Clarity
Clearly specified
Activity status
Active
Tech stack
github, typescript
Domain
backend-api-design, testing

Research direction

Start with src/lib/workflow/review-residue.post.test.ts, then read the referenced paths in review-residue.post.ts and batch-executor.ts to see how the post body and state-record fallback are handled. Add tests for a PR-body SEQUANT_MUTATION marker and for open versus closed or merged state-record PRs; run the listed Vitest commands and npm run lint:health to confirm the cases pass.

Written by the indexing model from the issue text.

Description

enhancement

Follow-up of #1520 (PR #1587); QA 3's two fix_now items.

Problem

Two runner-written paths in the Read-first post have no unit test. Both were exercised only by the testbed runs (ad-motion #290, #291) and by fake-provider cases that never reach them:

  1. deliverReadFirstComment (src/lib/workflow/review-residue.post.ts:190) reads the PR body through provider.fetchIssue(String(prNumber)) and merges its SEQUANT_MUTATION records with this run's exec output (mergeMarkers, :151). No fake PR body in review-residue.post.test.ts carries a marker, so a regression that drops the PR-body source would render mutation record: none on every qa-only re-run and no test would notice.
  2. resolveReadFirstPR (src/lib/workflow/batch-executor.ts:3037) falls back to the state record's PR and posts only when provider.getPRStatus says it is open. The post tests pass services.stateManager: null and a fake whose getPRStatus always answers open, so neither the fallback nor the guard runs under test.

Acceptance Criteria

  • AC-1: A qa-only run whose fake PR body carries a SEQUANT_MUTATION record renders mutation record: AC-N for the named test file in the posted comment, with no exec output in the run. Evidence: npx vitest run src/lib/workflow/review-residue.post.test.ts -t "PR body" → ≥ 1 passed, 0 failed (new case; today the filter matches 0 of 6 tests)
  • AC-1 is classified as a gate test by isGateTestEvidence; the PR body carries its SEQUANT_MUTATION record (remove the PR-body source from mergeMarkers's inputs → the new case fails; restore).
  • AC-2: With no post-QA PR and no early PR, a run whose state record names an open PR posts on it, and a run whose state record names a closed or merged PR posts nothing. The case constructs a real StateManager on a temp dir (or an in-memory one) with a pr record and a fake whose getPRStatus answers each state. Evidence: npx vitest run src/lib/workflow/review-residue.post.test.ts -t "state record" → ≥ 2 passed, 0 failed (new cases; today the filter matches 0 of 6 tests)
  • AC-2 is classified as a gate test by isGateTestEvidence; the PR body carries its SEQUANT_MUTATION record (remove the status.state === "open" check in resolveReadFirstPR → the closed-PR case fails; restore).
  • AC-3: The existing six cases still pass and no new test mocks child_process. Evidence: npx vitest run src/lib/workflow/review-residue.post.test.ts → ≥ 9 passed, 0 failed and npm run lint:health → no metric rose above the baseline

May touch

src/lib/workflow/review-residue.post.test.ts (and a small fixture if the PR body is long). No production change is expected; if one is needed to make a seam testable, it stays in review-residue.post.ts or batch-executor.ts's resolveReadFirstPR.

Non-Goals

  • Changing what the comment renders or where it posts (ADR-0021).
  • No edits under plugin/, .claude/skills/, templates/skills/ (plugin freeze).
Dominant language
TypeScript
Stars
1
Forks
1
Avg merge
11h 3m
Merged PRs (30d)
84

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from sequant-io/sequant

All issues in sequant-io/sequant

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.