refactor(git): one runGit helper in git-diff-utils replaces the four local git() spawnSync wrappers

Open
  • Is something wrong?
#1,592 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
72/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
git, node.js, typescript
Domain
testing, tooling

Research direction

Read src/lib/workflow/git-diff-utils.ts and its test, then trace the four wrapper sites listed in the issue. Add the shared helper tests for success and failure, and run the listed Vitest commands plus npm run lint && npm run lint:health. Done means all four wrappers are replaced, caller failure behavior is preserved, and the checks pass.

Written by the indexing model from the issue text.

Description

refactor

Follow-up of #1520 (PR #1587); from the independent code review. This is the synchronous consolidation; the async move is #506 and stays there.

Problem

Four modules carry their own function git( wrapper over spawnSync("git", …): src/lib/skills-commit-state.ts:36 (returns null on failure), src/lib/worktree-isolation.ts:102 (takes the command as one string), src/lib/merge-check/combined-branch-test.ts:70 and src/lib/workflow/review-residue.post.ts:28 (both throw, with different error text; the last one picks a 64 MB maxBuffer, the others the default). grep -lE '^function git\(' <the four files> | wc -l → 4 today. Each copy drifts on its own: a change to how a failed git read is reported, or to the buffer a large diff needs, lands in one and not the others. src/lib/workflow/git-diff-utils.ts already hosts the shared git reads (resolveDiffBase, getCommitHash, getGitDiffStats) and is where one helper belongs.

Acceptance Criteria

  • AC-1: One exported helper in git-diff-utils.ts (for example runGit(cwd, args, { maxBuffer? }), throwing with one error shape) replaces the four local wrappers; each former site calls it and keeps its own failure policy at the call site (catch and return null where it did). Evidence: grep -lE '^function git\(' src/lib/skills-commit-state.ts src/lib/worktree-isolation.ts src/lib/merge-check/combined-branch-test.ts src/lib/workflow/review-residue.post.ts | wc -l | tr -d ' ' → 0 (today 4)
  • AC-2: The helper has its own test for the two outcomes (stdout on success; a thrown error that names the args and stderr on failure). Evidence: npx vitest run src/lib/workflow/git-diff-utils.test.ts -t "runGit" → ≥ 2 passed, 0 failed (new cases)
  • AC-3: The four callers' tests still pass. Evidence: npx vitest run src/lib/skills-commit-state.integration.test.ts src/lib/worktree-isolation.test.ts src/lib/merge-check/combined-branch-test.test.ts src/lib/workflow/review-residue.post.test.ts → 0 failed
  • AC-4: No function grows past the #1508 ratchet and no metric rises. Evidence: npm run lint && npm run lint:health → exit 0, no metric rose above the baseline

May touch

src/lib/workflow/git-diff-utils.ts and its test, the four listed files, CHANGELOG.

Non-Goals

  • Making any of these async (#506).
  • Touching GitHubProvider or any gh call (#1506).
  • 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.