From 159c8b0c4d1f778af54d0428fd926ac767c26673 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Mon, 11 May 2026 03:38:53 +0200 Subject: [PATCH] =?UTF-8?q?refactor(git-service):=20rename=20GitServiceImp?= =?UTF-8?q?l=20=E2=86=92=20GitService?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- ARCHITECTURE.md | 59 +++++++++++++++++-- src/resources/extensions/sf/auto-start.js | 6 +- src/resources/extensions/sf/auto.js | 6 +- src/resources/extensions/sf/git-service.js | 8 +-- src/resources/extensions/sf/quick.js | 6 +- .../sf/tests/auto-post-unit-staging.test.mjs | 24 ++++---- .../sf/tests/worktree-fixes.test.mjs | 2 +- .../extensions/sf/worktree-detect.js | 4 +- .../extensions/sf/worktree-resolver.js | 2 +- src/resources/extensions/sf/worktree.js | 14 ++--- 10 files changed, 91 insertions(+), 40 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dfeb834a3..b22120756 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -96,13 +96,64 @@ stateDiagram-v2 PhaseComplete --> PhaseExecute : remediation milestone added note right of PhaseExecute - Task lifecycle (ORCH-style): - todo → running → verifying → reviewing - → done | blocked | paused | failed - | cancelled | retrying + See Task Lifecycle diagram below. 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 stateDiagram-v2 direction LR diff --git a/src/resources/extensions/sf/auto-start.js b/src/resources/extensions/sf/auto-start.js index 0edc23b41..9325c5859 100644 --- a/src/resources/extensions/sf/auto-start.js +++ b/src/resources/extensions/sf/auto-start.js @@ -39,7 +39,7 @@ import { setLevelChangeCallback, } from "./doctor-proactive.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 { initMetrics } from "./metrics.js"; import { initMetricsCentral } from "./metrics-central.js"; @@ -474,8 +474,8 @@ export async function bootstrapAutoSession( ); } } - // Initialize GitServiceImpl - s.gitService = new GitServiceImpl( + // Initialize GitService + s.gitService = new GitService( s.basePath, loadEffectiveSFPreferences()?.preferences?.git ?? {}, ); diff --git a/src/resources/extensions/sf/auto.js b/src/resources/extensions/sf/auto.js index 2025ccca0..c29352ca5 100644 --- a/src/resources/extensions/sf/auto.js +++ b/src/resources/extensions/sf/auto.js @@ -121,7 +121,7 @@ import { setLevelChangeCallback, } from "./doctor-proactive.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 { emitJournalEvent as _emitJournalEvent } from "./journal.js"; import { @@ -1304,7 +1304,7 @@ function buildResolverDeps() { autoWorktreeBranch, resolveMilestoneFile, readFileSync: (path, encoding) => readFileSync(path, encoding), - GitServiceImpl: GitServiceImpl, + GitService: GitService, loadEffectiveSFPreferences: loadEffectiveSFPreferences, invalidateAllCaches, captureIntegrationBranch, @@ -1406,7 +1406,7 @@ function buildLoopDeps() { readFileSync: (path, encoding) => readFileSync(path, encoding), atomicWriteSync, // Git - GitServiceImpl: GitServiceImpl, + GitService: GitService, // WorktreeResolver resolver: buildResolver(), // Post-unit processing diff --git a/src/resources/extensions/sf/git-service.js b/src/resources/extensions/sf/git-service.js index 68a469bce..42963609e 100644 --- a/src/resources/extensions/sf/git-service.js +++ b/src/resources/extensions/sf/git-service.js @@ -374,8 +374,8 @@ const COMMIT_TYPE_RULES = [ "chore", ], ]; -// ─── GitServiceImpl ──────────────────────────────────────────────────── -export class GitServiceImpl { +// ─── GitService ──────────────────────────────────────────────────── +export class GitService { basePath; prefs; /** Active milestone ID — used to resolve the integration branch. */ @@ -766,10 +766,10 @@ export function createDraftPR(basePath, _milestoneId, title, body, opts) { } } // ─── Factory ─────────────────────────────────────────────────────────────── -/** Create a GitServiceImpl with the current effective git preferences. */ +/** Create a GitService with the current effective git preferences. */ export function createGitService(basePath) { const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; - return new GitServiceImpl(basePath, gitPrefs); + return new GitService(basePath, gitPrefs); } function buildTurnSnapshotLabel(unitType, unitId) { const raw = `${unitType}/${unitId}`.trim(); diff --git a/src/resources/extensions/sf/quick.js b/src/resources/extensions/sf/quick.js index 6062d9028..1c6f91356 100644 --- a/src/resources/extensions/sf/quick.js +++ b/src/resources/extensions/sf/quick.js @@ -17,7 +17,7 @@ import { writeFileSync, } from "node:fs"; 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 { sfRoot } from "./paths.js"; import { loadEffectiveSFPreferences } from "./preferences.js"; @@ -117,7 +117,7 @@ export function cleanupQuickBranch(basePath = process.cwd()) { if (!state) return false; const repoPath = state.basePath; const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; - const git = new GitServiceImpl(repoPath, gitPrefs); + const git = new GitService(repoPath, gitPrefs); if (git.getCurrentBranch() === state.quickBranch) { try { 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]; // Create git branch for the quick task (unless isolation:none — #3337) const gitPrefs = loadEffectiveSFPreferences()?.preferences?.git ?? {}; - const git = new GitServiceImpl(basePath, gitPrefs); + const git = new GitService(basePath, gitPrefs); const branchName = `sf/quick/${taskNum}-${slug}`; const originalBranch = git.getCurrentBranch(); const { getIsolationMode } = await import("./preferences.js"); diff --git a/src/resources/extensions/sf/tests/auto-post-unit-staging.test.mjs b/src/resources/extensions/sf/tests/auto-post-unit-staging.test.mjs index 5b6560104..e54801568 100644 --- a/src/resources/extensions/sf/tests/auto-post-unit-staging.test.mjs +++ b/src/resources/extensions/sf/tests/auto-post-unit-staging.test.mjs @@ -49,8 +49,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { writeFileSync(join(base, "src/index.ts"), "export {}"); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); git.stageExplicitIncludePaths(["src/index.ts"], []); expect(nativeAddPathsMock).toHaveBeenCalledOnce(); @@ -68,8 +68,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan"); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); git.stageExplicitIncludePaths([".sf/plans/foo.md"], []); expect(nativeAddPathsMock).not.toHaveBeenCalled(); @@ -89,8 +89,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { ); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); git.stageExplicitIncludePaths([".sf/milestones/M001/SLICE.md"], []); expect(nativeAddPathsMock).not.toHaveBeenCalled(); @@ -109,8 +109,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { writeFileSync(join(base, ".sf", "plans", "foo.md"), "# plan"); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); git.stageExplicitIncludePaths([".sf/plans/foo.md", "src/index.ts"], []); expect(nativeAddPathsMock).toHaveBeenCalledOnce(); @@ -126,8 +126,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { mkdirSync(join(base, ".git"), { recursive: true }); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); // Windows-style: .sf\plans\foo.md → normalized to .sf/plans/foo.md // First segment is .sf → should be filtered git.stageExplicitIncludePaths([".sf\\plans\\foo.md"], []); @@ -146,8 +146,8 @@ describe("stageExplicitIncludePaths .sf/ filter", () => { writeFileSync(join(base, "foo", ".sf", "plans", "foo.md"), "# plan"); nativeAddPathsMock.mockClear(); - const { GitServiceImpl } = await import("../git-service.js"); - const git = new GitServiceImpl(base); + const { GitService } = await import("../git-service.js"); + const git = new GitService(base); // foo\.sf\... → normalized to foo/.sf/... → first segment = foo → NOT filtered // (this is a subdirectory named .sf under foo/, not the SF state dir) git.stageExplicitIncludePaths(["foo\\.sf\\plans\\foo.md"], []); diff --git a/src/resources/extensions/sf/tests/worktree-fixes.test.mjs b/src/resources/extensions/sf/tests/worktree-fixes.test.mjs index fb49aeff0..209bd095a 100644 --- a/src/resources/extensions/sf/tests/worktree-fixes.test.mjs +++ b/src/resources/extensions/sf/tests/worktree-fixes.test.mjs @@ -232,7 +232,7 @@ describe("WorktreeResolver.enterMilestone", () => { getAutoWorktreePath: () => "/project/.sf/worktrees/M001", enterAutoWorktree: () => "/project/.sf/worktrees/M001", createAutoWorktree: () => "/project/.sf/worktrees/M001", - GitServiceImpl: class MockGitService {}, + GitService: class MockGitService {}, loadEffectiveSFPreferences: () => ({}), invalidateAllCaches: () => {}, }; diff --git a/src/resources/extensions/sf/worktree-detect.js b/src/resources/extensions/sf/worktree-detect.js index bb43a5dbe..eb4385cbb 100644 --- a/src/resources/extensions/sf/worktree-detect.js +++ b/src/resources/extensions/sf/worktree-detect.js @@ -3,7 +3,7 @@ * * Extracted from worktree.js so that git-service.js can import * 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. * * 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. * Returns null if not inside a SF worktree (.sf/worktrees//). * - * Consumer: git-service.js (GitServiceImpl.getTargetBranch) and worktree.js. + * Consumer: git-service.js (GitService.getTargetBranch) and worktree.js. * * @param {string} basePath * @returns {string | null} diff --git a/src/resources/extensions/sf/worktree-resolver.js b/src/resources/extensions/sf/worktree-resolver.js index 7b87ce993..ce31badad 100644 --- a/src/resources/extensions/sf/worktree-resolver.js +++ b/src/resources/extensions/sf/worktree-resolver.js @@ -58,7 +58,7 @@ export class WorktreeResolver { rebuildGitService() { const gitConfig = this.deps.loadEffectiveSFPreferences()?.preferences?.git ?? {}; - this.s.gitService = new this.deps.GitServiceImpl( + this.s.gitService = new this.deps.GitService( this.s.basePath, gitConfig, ); diff --git a/src/resources/extensions/sf/worktree.js b/src/resources/extensions/sf/worktree.js index bc2fce3e3..a4c29a1aa 100644 --- a/src/resources/extensions/sf/worktree.js +++ b/src/resources/extensions/sf/worktree.js @@ -14,7 +14,7 @@ import { existsSync, readFileSync, realpathSync, utimesSync } from "node:fs"; import { homedir } from "node:os"; 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 { 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 export { detectWorktreeName } from "./worktree-detect.js"; -// ─── Lazy GitServiceImpl Cache ───────────────────────────────────────────── +// ─── Lazy GitService Cache ───────────────────────────────────────────── let cachedService = 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. * Lazy construction: only instantiated at call-time, never at module-evaluation. */ @@ -34,13 +34,13 @@ function getService(basePath) { if (cachedService === null || cachedBasePath !== basePath) { const loaded = loadEffectiveSFPreferences(); const gitPrefs = loaded?.preferences?.git ?? {}; - cachedService = new GitServiceImpl(basePath, gitPrefs); + cachedService = new GitService(basePath, gitPrefs); cachedBasePath = basePath; } 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. * @internal */ @@ -49,7 +49,7 @@ export function _resetServiceCache() { 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(). */ export function setActiveMilestoneId(basePath, milestoneId) { @@ -215,7 +215,7 @@ export function parseSliceBranch(branchName) { sliceId: match[3], }; } -// ─── Git-Mutation Functions (delegate to GitServiceImpl) ─────────────────── +// ─── Git-Mutation Functions (delegate to GitService) ─────────────────── /** * Get the "main" branch for SF slice operations. *