refactor(workflow): no test seam defaults to a live GitHubProvider — one required provider on the issue context, set once where the context is built

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

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
node.js, typescript
Domain
backend, testing

Research direction

Start with src/lib/workflow/types.ts and src/lib/workflow/batch-executor.ts to trace how IssueExecutionContext gets its providers, then follow context construction in run-orchestrator.ts and the CLI entry. Update the tests that set the optional seams and run the four Vitest files named in AC-2 plus npx tsc --noEmit -p tsconfig.json. Done when the context requires one provider, no inline GitHubProvider remains in batch-executor.ts, and the listed checks pass.

Written by the indexing model from the issue text.

Description

refactor

Follow-up of #1520 (PR #1587); from the independent code review. Not a #1520 defect: the new readFirstProvider seam copies the pattern the existing postComment seam set.

Problem

runIssueWithLogging reaches GitHub through optional test seams on IssueExecutionContext (src/lib/workflow/types.ts:985 postComment?, :990 readFirstProvider?). When a caller leaves one unset, batch-executor.ts constructs a live GitHubProvider on the spot: grep -c 'new GitHubProvider()' src/lib/workflow/batch-executor.ts → 6 today (lines 333 and 388 as parameter defaults, 979, 1067, 1071 and 3074 inline). Every runIssueWithLogging test with a qa phase now passes through postReadFirstForPR (:3066); the only thing between a test that forgets the seam and a real gh issue view / gh pr comment against the developer's authenticated gh is the existsSync(worktreePath) check. Today's real-repo integration tests mock runIssueWithLogging, so the exposure is latent. #1506 moved every gh call behind PlatformProvider; this is the remaining way a test can still reach gh by omission.

A harness guard (refuse to post when VITEST is set) is not the fix: it would be a workaround for a construction problem, and the owner's standing rule reads a harness workaround as a bug pointer.

Acceptance Criteria

  • AC-1: batch-executor.ts constructs no GitHubProvider inline. The provider is one required field of the context (or the services record), set once where the context is built (run-orchestrator.ts / the CLI entry) and passed down. Evidence: grep -c 'new GitHubProvider()' src/lib/workflow/batch-executor.ts → 0 (today 6)
  • AC-2: The postComment and readFirstProvider seams are gone or derive from that one provider, and every existing test that set them still passes. Evidence: grep -cE 'postComment\?:|readFirstProvider\?:' src/lib/workflow/types.ts → 0 (today 2), and npx vitest run src/lib/workflow/batch-executor.test.ts src/lib/workflow/batch-executor.model-ladder.test.ts src/lib/workflow/batch-executor.suppressed.integration.test.ts src/lib/workflow/review-residue.post.test.ts → 0 failed
  • AC-3: A runIssueWithLogging call with no provider is a type error, not a live call. Evidence: npx tsc --noEmit -p tsconfig.json → exit 0 after the change, and a // @ts-expect-error line in a test that omits the provider compiles (shows the omission is rejected)
  • AC-4: Behaviour on a real run is unchanged: the QA verdict comment and the Read-first comment still post. Evidence: one sequant run on the ad-motion #285 testbed ("(run 7)") shows the verdict comment on the issue and gh pr view "$PR" --json comments --jq '[.comments[].body | select(startswith("## Read first"))] | length' → 1

May touch

src/lib/workflow/types.ts, batch-executor.ts, run-orchestrator.ts, the tests that build an IssueExecutionContext, CHANGELOG.

Non-Goals

  • Any env-based guard in production code.
  • Changing PlatformProvider's interface (#1506 owns it).
  • 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.