refactor(git-service): rename GitServiceImpl → GitService

No interface exists for the class, so the Impl suffix is vestigial
Java-style naming. Rename throughout: git-service.js, auto-start.js,
auto.js, worktree.js, worktree-detect.js, worktree-resolver.js,
quick.js, and the two test files that imported it directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mikael Hugo 2026-05-11 03:38:53 +02:00
parent c1df4249b8
commit 159c8b0c4d
10 changed files with 91 additions and 40 deletions

View file

@ -96,13 +96,64 @@ stateDiagram-v2
PhaseComplete --> PhaseExecute : remediation milestone added PhaseComplete --> PhaseExecute : remediation milestone added
note right of PhaseExecute note right of PhaseExecute
Task lifecycle (ORCH-style): See Task Lifecycle diagram below.
todo → running → verifying → reviewing
→ done | blocked | paused | failed
| cancelled | retrying
end note end note
``` ```
```mermaid
stateDiagram-v2
direction TB
[*] --> todo : task created
todo --> running : dispatch picks task
todo --> cancelled : explicit cancel
running --> verifying : implementation done, run checks
running --> reviewing : needs human / agent review
running --> done : trivial task, skip verify
running --> blocked : dependency unresolved
running --> paused : user interrupt
running --> retrying : transient failure, retry
running --> failed : unrecoverable error
running --> cancelled : explicit cancel
verifying --> reviewing : checks pass, review needed
verifying --> done : checks pass, no review needed
verifying --> blocked : check dependency missing
verifying --> paused : user interrupt
verifying --> retrying : check flake, retry
verifying --> failed : checks failed
verifying --> cancelled : explicit cancel
reviewing --> running : feedback applied, re-implement
reviewing --> verifying : back to verify after edits
reviewing --> done : review approved
reviewing --> blocked : waiting on reviewer
reviewing --> paused : user interrupt
reviewing --> failed : review rejected
reviewing --> cancelled : explicit cancel
blocked --> todo : dependency resolved, reset
blocked --> running : unblocked, resume
blocked --> retrying : auto-unblock retry
blocked --> cancelled : explicit cancel
paused --> running : resume
paused --> retrying : auto-resume
paused --> cancelled : explicit cancel
retrying --> running : retry attempt starts
retrying --> failed : retry budget exhausted
retrying --> cancelled : explicit cancel
failed --> retrying : manual re-queue
failed --> cancelled : give up
done --> [*]
cancelled --> [*]
```
```mermaid ```mermaid
stateDiagram-v2 stateDiagram-v2
direction LR direction LR

View file

@ -39,7 +39,7 @@ import {
setLevelChangeCallback, setLevelChangeCallback,
} from "./doctor-proactive.js"; } from "./doctor-proactive.js";
import { getManifestStatus, loadFile } from "./files.js"; import { getManifestStatus, loadFile } from "./files.js";
import { GitServiceImpl } from "./git-service.js"; import { GitService } from "./git-service.js";
import { ensureGitignore, untrackRuntimeFiles } from "./gitignore.js"; import { ensureGitignore, untrackRuntimeFiles } from "./gitignore.js";
import { initMetrics } from "./metrics.js"; import { initMetrics } from "./metrics.js";
import { initMetricsCentral } from "./metrics-central.js"; import { initMetricsCentral } from "./metrics-central.js";
@ -474,8 +474,8 @@ export async function bootstrapAutoSession(
); );
} }
} }
// Initialize GitServiceImpl // Initialize GitService
s.gitService = new GitServiceImpl( s.gitService = new GitService(
s.basePath, s.basePath,
loadEffectiveSFPreferences()?.preferences?.git ?? {}, loadEffectiveSFPreferences()?.preferences?.git ?? {},
); );

View file

@ -121,7 +121,7 @@ import {
setLevelChangeCallback, setLevelChangeCallback,
} from "./doctor-proactive.js"; } from "./doctor-proactive.js";
import { getErrorMessage } from "./error-utils.js"; import { getErrorMessage } from "./error-utils.js";
import { GitServiceImpl } from "./git-service.js"; import { GitService } from "./git-service.js";
import { initHealthWidget } from "./health-widget.js"; import { initHealthWidget } from "./health-widget.js";
import { emitJournalEvent as _emitJournalEvent } from "./journal.js"; import { emitJournalEvent as _emitJournalEvent } from "./journal.js";
import { import {
@ -1304,7 +1304,7 @@ function buildResolverDeps() {
autoWorktreeBranch, autoWorktreeBranch,
resolveMilestoneFile, resolveMilestoneFile,
readFileSync: (path, encoding) => readFileSync(path, encoding), readFileSync: (path, encoding) => readFileSync(path, encoding),
GitServiceImpl: GitServiceImpl, GitService: GitService,
loadEffectiveSFPreferences: loadEffectiveSFPreferences, loadEffectiveSFPreferences: loadEffectiveSFPreferences,
invalidateAllCaches, invalidateAllCaches,
captureIntegrationBranch, captureIntegrationBranch,
@ -1406,7 +1406,7 @@ function buildLoopDeps() {
readFileSync: (path, encoding) => readFileSync(path, encoding), readFileSync: (path, encoding) => readFileSync(path, encoding),
atomicWriteSync, atomicWriteSync,
// Git // Git
GitServiceImpl: GitServiceImpl, GitService: GitService,
// WorktreeResolver // WorktreeResolver
resolver: buildResolver(), resolver: buildResolver(),
// Post-unit processing // Post-unit processing

View file

@ -374,8 +374,8 @@ const COMMIT_TYPE_RULES = [
"chore", "chore",
], ],
]; ];
// ─── GitServiceImpl ──────────────────────────────────────────────────── // ─── GitService ────────────────────────────────────────────────────
export class GitServiceImpl { export class GitService {
basePath; basePath;
prefs; prefs;
/** Active milestone ID — used to resolve the integration branch. */ /** Active milestone ID — used to resolve the integration branch. */
@ -766,10 +766,10 @@ export function createDraftPR(basePath, _milestoneId, title, body, opts) {
} }
} }
// ─── Factory ─────────────────────────────────────────────────────────────── // ─── Factory ───────────────────────────────────────────────────────────────
/** Create a GitServiceImpl with the current effective git preferences. */ /** Create a GitService with the current effective git preferences. */
export function createGitService(basePath) { export function createGitService(basePath) {
const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {};
return new GitServiceImpl(basePath, gitPrefs); return new GitService(basePath, gitPrefs);
} }
function buildTurnSnapshotLabel(unitType, unitId) { function buildTurnSnapshotLabel(unitType, unitId) {
const raw = `${unitType}/${unitId}`.trim(); const raw = `${unitType}/${unitId}`.trim();

View file

@ -17,7 +17,7 @@ import {
writeFileSync, writeFileSync,
} from "node:fs"; } from "node:fs";
import { join } from "node:path"; import { join } from "node:path";
import { GitServiceImpl, runGit } from "./git-service.js"; import { GitService, runGit } from "./git-service.js";
import { nativeHasStagedChanges } from "./native-git-bridge.js"; import { nativeHasStagedChanges } from "./native-git-bridge.js";
import { sfRoot } from "./paths.js"; import { sfRoot } from "./paths.js";
import { loadEffectiveSFPreferences } from "./preferences.js"; import { loadEffectiveSFPreferences } from "./preferences.js";
@ -117,7 +117,7 @@ export function cleanupQuickBranch(basePath = process.cwd()) {
if (!state) return false; if (!state) return false;
const repoPath = state.basePath; const repoPath = state.basePath;
const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {};
const git = new GitServiceImpl(repoPath, gitPrefs); const git = new GitService(repoPath, gitPrefs);
if (git.getCurrentBranch() === state.quickBranch) { if (git.getCurrentBranch() === state.quickBranch) {
try { try {
git.autoCommit("quick-task", `Q${state.taskNum}`, []); git.autoCommit("quick-task", `Q${state.taskNum}`, []);
@ -170,7 +170,7 @@ export async function handleQuick(args, ctx, pi) {
const date = new Date().toISOString().split("T")[0]; const date = new Date().toISOString().split("T")[0];
// Create git branch for the quick task (unless isolation:none — #3337) // Create git branch for the quick task (unless isolation:none — #3337)
const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {};
const git = new GitServiceImpl(basePath, gitPrefs); const git = new GitService(basePath, gitPrefs);
const branchName = `sf/quick/${taskNum}-${slug}`; const branchName = `sf/quick/${taskNum}-${slug}`;
const originalBranch = git.getCurrentBranch(); const originalBranch = git.getCurrentBranch();
const { getIsolationMode } = await import("./preferences.js"); const { getIsolationMode } = await import("./preferences.js");

View file

@ -49,8 +49,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
writeFileSync(join(base, "src/index.ts"), "export {}"); writeFileSync(join(base, "src/index.ts"), "export {}");
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
git.stageExplicitIncludePaths(["src/index.ts"], []); git.stageExplicitIncludePaths(["src/index.ts"], []);
expect(nativeAddPathsMock).toHaveBeenCalledOnce(); expect(nativeAddPathsMock).toHaveBeenCalledOnce();
@ -68,8 +68,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan"); writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan");
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
git.stageExplicitIncludePaths([".sf/plans/foo.md"], []); git.stageExplicitIncludePaths([".sf/plans/foo.md"], []);
expect(nativeAddPathsMock).not.toHaveBeenCalled(); expect(nativeAddPathsMock).not.toHaveBeenCalled();
@ -89,8 +89,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
); );
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
git.stageExplicitIncludePaths([".sf/milestones/M001/SLICE.md"], []); git.stageExplicitIncludePaths([".sf/milestones/M001/SLICE.md"], []);
expect(nativeAddPathsMock).not.toHaveBeenCalled(); expect(nativeAddPathsMock).not.toHaveBeenCalled();
@ -109,8 +109,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan"); writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan");
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
git.stageExplicitIncludePaths([".sf/plans/foo.md", "src/index.ts"], []); git.stageExplicitIncludePaths([".sf/plans/foo.md", "src/index.ts"], []);
expect(nativeAddPathsMock).toHaveBeenCalledOnce(); expect(nativeAddPathsMock).toHaveBeenCalledOnce();
@ -126,8 +126,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
mkdirSync(join(base, ".git"), { recursive: true }); mkdirSync(join(base, ".git"), { recursive: true });
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
// Windows-style: .sf\plans\foo.md → normalized to .sf/plans/foo.md // Windows-style: .sf\plans\foo.md → normalized to .sf/plans/foo.md
// First segment is .sf → should be filtered // First segment is .sf → should be filtered
git.stageExplicitIncludePaths([".sf\\plans\\foo.md"], []); git.stageExplicitIncludePaths([".sf\\plans\\foo.md"], []);
@ -146,8 +146,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => {
writeFileSync(join(base, "foo", ".sf", "plans", "foo.md"), "# plan"); writeFileSync(join(base, "foo", ".sf", "plans", "foo.md"), "# plan");
nativeAddPathsMock.mockClear(); nativeAddPathsMock.mockClear();
const { GitServiceImpl } = await import("../git-service.js"); const { GitService } = await import("../git-service.js");
const git = new GitServiceImpl(base); const git = new GitService(base);
// foo\.sf\... → normalized to foo/.sf/... → first segment = foo → NOT filtered // foo\.sf\... → normalized to foo/.sf/... → first segment = foo → NOT filtered
// (this is a subdirectory named .sf under foo/, not the SF state dir) // (this is a subdirectory named .sf under foo/, not the SF state dir)
git.stageExplicitIncludePaths(["foo\\.sf\\plans\\foo.md"], []); git.stageExplicitIncludePaths(["foo\\.sf\\plans\\foo.md"], []);

View file

@ -232,7 +232,7 @@ describe("WorktreeResolver.enterMilestone", () => {
getAutoWorktreePath: () => "/project/.sf/worktrees/M001", getAutoWorktreePath: () => "/project/.sf/worktrees/M001",
enterAutoWorktree: () => "/project/.sf/worktrees/M001", enterAutoWorktree: () => "/project/.sf/worktrees/M001",
createAutoWorktree: () => "/project/.sf/worktrees/M001", createAutoWorktree: () => "/project/.sf/worktrees/M001",
GitServiceImpl: class MockGitService {}, GitService: class MockGitService {},
loadEffectiveSFPreferences: () => ({}), loadEffectiveSFPreferences: () => ({}),
invalidateAllCaches: () => {}, invalidateAllCaches: () => {},
}; };

View file

@ -3,7 +3,7 @@
* *
* Extracted from worktree.js so that git-service.js can import * Extracted from worktree.js so that git-service.js can import
* detectWorktreeName without pulling in the full worktree module * detectWorktreeName without pulling in the full worktree module
* (which in turn imports GitServiceImpl from git-service.js, creating * (which in turn imports GitService from git-service.js, creating
* a cycle). worktree.js re-exports both symbols for back-compat. * a cycle). worktree.js re-exports both symbols for back-compat.
* *
* Purpose: provide a zero-dependency path for detecting whether a * Purpose: provide a zero-dependency path for detecting whether a
@ -40,7 +40,7 @@ function findWorktreeSegment(normalizedPath) {
* Detect the active worktree name from the current working directory. * Detect the active worktree name from the current working directory.
* Returns null if not inside a SF worktree (.sf/worktrees/<name>/). * Returns null if not inside a SF worktree (.sf/worktrees/<name>/).
* *
* Consumer: git-service.js (GitServiceImpl.getTargetBranch) and worktree.js. * Consumer: git-service.js (GitService.getTargetBranch) and worktree.js.
* *
* @param {string} basePath * @param {string} basePath
* @returns {string | null} * @returns {string | null}

View file

@ -58,7 +58,7 @@ export class WorktreeResolver {
rebuildGitService() { rebuildGitService() {
const gitConfig = const gitConfig =
this.deps.loadEffectiveSFPreferences()?.preferences?.git ?? {}; this.deps.loadEffectiveSFPreferences()?.preferences?.git ?? {};
this.s.gitService = new this.deps.GitServiceImpl( this.s.gitService = new this.deps.GitService(
this.s.basePath, this.s.basePath,
gitConfig, gitConfig,
); );

View file

@ -14,7 +14,7 @@
import { existsSync, readFileSync, realpathSync, utimesSync } from "node:fs"; import { existsSync, readFileSync, realpathSync, utimesSync } from "node:fs";
import { homedir } from "node:os"; import { homedir } from "node:os";
import { join, resolve } from "node:path"; import { join, resolve } from "node:path";
import { GitServiceImpl, writeIntegrationBranch } from "./git-service.js"; import { GitService, writeIntegrationBranch } from "./git-service.js";
import { loadEffectiveSFPreferences } from "./preferences.js"; import { loadEffectiveSFPreferences } from "./preferences.js";
import { detectWorktreeName, findWorktreeSegment } from "./worktree-detect.js"; import { detectWorktreeName, findWorktreeSegment } from "./worktree-detect.js";
@ -22,11 +22,11 @@ export { MergeConflictError } from "./git-service.js";
// Re-export for consumers that import detectWorktreeName from ./worktree.js // Re-export for consumers that import detectWorktreeName from ./worktree.js
export { detectWorktreeName } from "./worktree-detect.js"; export { detectWorktreeName } from "./worktree-detect.js";
// ─── Lazy GitServiceImpl Cache ───────────────────────────────────────────── // ─── Lazy GitService Cache ─────────────────────────────────────────────
let cachedService = null; let cachedService = null;
let cachedBasePath = null; let cachedBasePath = null;
/** /**
* Get or create a GitServiceImpl for the given basePath. * Get or create a GitService for the given basePath.
* Resets the cache if basePath changes between calls. * Resets the cache if basePath changes between calls.
* Lazy construction: only instantiated at call-time, never at module-evaluation. * Lazy construction: only instantiated at call-time, never at module-evaluation.
*/ */
@ -34,13 +34,13 @@ function getService(basePath) {
if (cachedService === null || cachedBasePath !== basePath) { if (cachedService === null || cachedBasePath !== basePath) {
const loaded = loadEffectiveSFPreferences(); const loaded = loadEffectiveSFPreferences();
const gitPrefs = loaded?.preferences?.git ?? {}; const gitPrefs = loaded?.preferences?.git ?? {};
cachedService = new GitServiceImpl(basePath, gitPrefs); cachedService = new GitService(basePath, gitPrefs);
cachedBasePath = basePath; cachedBasePath = basePath;
} }
return cachedService; return cachedService;
} }
/** /**
* Clear the cached GitServiceImpl. For testing only forces the next * Clear the cached GitService. For testing only forces the next
* getService() call to re-read preferences and create a fresh instance. * getService() call to re-read preferences and create a fresh instance.
* @internal * @internal
*/ */
@ -49,7 +49,7 @@ export function _resetServiceCache() {
cachedBasePath = null; cachedBasePath = null;
} }
/** /**
* Set the active milestone ID on the cached GitServiceImpl. * Set the active milestone ID on the cached GitService.
* This enables integration branch resolution in getMainBranch(). * This enables integration branch resolution in getMainBranch().
*/ */
export function setActiveMilestoneId(basePath, milestoneId) { export function setActiveMilestoneId(basePath, milestoneId) {
@ -215,7 +215,7 @@ export function parseSliceBranch(branchName) {
sliceId: match[3], sliceId: match[3],
}; };
} }
// ─── Git-Mutation Functions (delegate to GitServiceImpl) ─────────────────── // ─── Git-Mutation Functions (delegate to GitService) ───────────────────
/** /**
* Get the "main" branch for SF slice operations. * Get the "main" branch for SF slice operations.
* *