From 7aa463db5c71f7ae3f5d2720208a64b9d2b5ca9a Mon Sep 17 00:00:00 2001 From: Corwin Marsh Date: Wed, 1 Apr 2026 18:13:11 -0700 Subject: [PATCH] fix(status): refresh remote tracking before reporting branch state (#138) Fetch the tracked remote ref before computing ahead/behind counts while preserving local status output when refresh fails. Refs: https://github.com/corwinm/arashi-arashi/issues/138 --- src/commands/status.ts | 65 +++++++++++++++++++++++++-- src/lib/git-remote.ts | 92 ++++++++++++++++++++++++++++----------- tests/unit/status.test.ts | 89 +++++++++++++++++++++++++++++++++++++ 3 files changed, 216 insertions(+), 30 deletions(-) diff --git a/src/commands/status.ts b/src/commands/status.ts index 0d411d3..57663a7 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -6,6 +6,7 @@ * Supports three output modes: default, verbose, and short. */ +import { fetchRemoteTrackingTarget, resolveRemoteTrackingTarget } from "../lib/git-remote.js"; import { findWorkspaceRoot, loadConfig } from "../lib/config.js"; import { getFullGitStatus, getGitStatus } from "../lib/git.js"; import { info, error as logError, spinner } from "../lib/logger.js"; @@ -108,10 +109,31 @@ export interface RepoStatus { files: GitFileStatus[]; /** Error message if status check failed */ error: string | null; + /** Warning shown when remote tracking refresh fails but local status is still available */ + refreshWarning?: string | null; /** Full git status output (for verbose mode) */ fullStatus?: string; } +interface StatusCommandDependencies { + fetchRemoteTrackingTarget: typeof fetchRemoteTrackingTarget; + getFullGitStatus: typeof getFullGitStatus; + getGitStatus: typeof getGitStatus; + resolveRemoteTrackingTarget: typeof resolveRemoteTrackingTarget; +} + +interface CheckRepoStatusOptions { + dependencies?: StatusCommandDependencies; + verbose?: boolean; +} + +const defaultStatusCommandDependencies: StatusCommandDependencies = { + fetchRemoteTrackingTarget, + getFullGitStatus, + getGitStatus, + resolveRemoteTrackingTarget, +}; + /** * Parse git status porcelain output * @@ -209,11 +231,14 @@ const pathExists = async (path: string): Promise => { } }; +const createRefreshWarning = (error: string): string => `Remote tracking may be stale: ${error}`; + export const checkRepoStatus = async ( name: string, path: string, - verbose = false, + options: CheckRepoStatusOptions = {}, ): Promise => { + const { dependencies = defaultStatusCommandDependencies, verbose = false } = options; const repoExists = await pathExists(path); if (!repoExists) { return { @@ -222,11 +247,21 @@ export const checkRepoStatus = async ( files: [], name, path, + refreshWarning: null, }; } try { - const result = await getGitStatus(path); + let refreshWarning: string | null = null; + const trackingTarget = await dependencies.resolveRemoteTrackingTarget(path); + if (trackingTarget.ok) { + const fetchResult = await dependencies.fetchRemoteTrackingTarget(path, trackingTarget.target); + if (!fetchResult.ok) { + refreshWarning = createRefreshWarning(fetchResult.error); + } + } + + const result = await dependencies.getGitStatus(path); if (result.error) { return { @@ -235,6 +270,7 @@ export const checkRepoStatus = async ( files: [], name, path, + refreshWarning, }; } @@ -242,7 +278,7 @@ export const checkRepoStatus = async ( let fullStatus: string | undefined = undefined; if (verbose) { - const fullResult = await getFullGitStatus(path); + const fullResult = await dependencies.getFullGitStatus(path); if (fullResult.error) { fullStatus = fullResult.error; } else { @@ -257,6 +293,7 @@ export const checkRepoStatus = async ( fullStatus, name, path, + refreshWarning, }; } catch (error) { let errorMessage = "Unknown error"; @@ -270,6 +307,7 @@ export const checkRepoStatus = async ( files: [], name, path, + refreshWarning: null, }; } }; @@ -299,7 +337,9 @@ export const checkAllRepos = ( reposToCheck.push({ name, path: absolutePath }); } - const statusPromises = reposToCheck.map((repo) => checkRepoStatus(repo.name, repo.path, verbose)); + const statusPromises = reposToCheck.map((repo) => + checkRepoStatus(repo.name, repo.path, { verbose }), + ); return Promise.all(statusPromises); }; @@ -355,6 +395,11 @@ const getStatusColor = (isClean: boolean) => (hasError: boolean) => { */ const cyan = (text: string): string => `\u001B[36m${text}\u001B[0m`; +/** + * Helper to apply yellow color + */ +const yellow = (text: string): string => `\u001B[33m${text}\u001B[0m`; + /** * Helper to apply bold */ @@ -414,6 +459,10 @@ export const formatRepoSection = (status: RepoStatus): string => { section += ` ${parts.join(", ")}\n`; } + if (status.refreshWarning) { + section += ` ${yellow(`Warning: ${status.refreshWarning}`)}\n`; + } + return section; }; @@ -481,6 +530,10 @@ export const formatVerboseOutput = (statuses: RepoStatus[]): string => { const colorFn = getStatusColor(true)(false); output += ` ${colorFn("✓ Clean - No changes")}\n`; } + + if (status.refreshWarning) { + output += ` ${yellow(`Warning: ${status.refreshWarning}`)}\n`; + } } output += formatSummary(statuses); @@ -543,6 +596,10 @@ export const formatShortLine = (status: RepoStatus): string => { line += colorFn(`● ${status.files.length} changes (${parts.join(", ")})`); } + if (status.refreshWarning) { + line += ` ${yellow("(remote tracking stale)")}`; + } + return line; }; diff --git a/src/lib/git-remote.ts b/src/lib/git-remote.ts index 5de9549..b70697c 100644 --- a/src/lib/git-remote.ts +++ b/src/lib/git-remote.ts @@ -1,5 +1,15 @@ import { exec } from "./git.ts"; +export interface RemoteTrackingTarget { + upstream: string | null; + remote: string; + branch: string; +} + +export type RemoteTrackingTargetResolution = + | { ok: true; target: RemoteTrackingTarget } + | { ok: false; error: string; upstream: string | null }; + export interface RemoteChangeStatus { repositoryId: string; upstream: string | null; @@ -11,10 +21,9 @@ export interface RemoteChangeStatus { error?: string; } -export async function checkRemoteChanges( - repositoryId: string, +export async function resolveRemoteTrackingTarget( repoPath: string, -): Promise { +): Promise { let upstream: string | null = null; let remote: string | null = null; let branch: string | null = null; @@ -37,53 +46,84 @@ export async function checkRemoteChanges( if (!remote || !branch) { const fallback = await resolveRemoteAndBranch(repoPath); if (!fallback.ok) { - return { - ahead: 0, - behind: 0, - branch: null, - error: fallback.error, - hasRemoteChanges: false, - remote: null, - repositoryId, - upstream, - }; + return { error: fallback.error, ok: false, upstream }; } ({ remote } = fallback); ({ branch } = fallback); } if (!remote || !branch) { + return { error: "Unable to determine remote tracking branch", ok: false, upstream }; + } + + return { + ok: true, + target: { + branch, + remote, + upstream, + }, + }; +} + +export async function fetchRemoteTrackingTarget( + repoPath: string, + target: RemoteTrackingTarget, +): Promise<{ ok: true } | { ok: false; error: string }> { + try { + await exec( + [ + "fetch", + "--prune", + target.remote, + `+refs/heads/${target.branch}:refs/remotes/${target.remote}/${target.branch}`, + ], + repoPath, + ); + return { ok: true }; + } catch (error) { + return { + error: error instanceof Error ? error.message : "Remote fetch failed", + ok: false, + }; + } +} + +export async function checkRemoteChanges( + repositoryId: string, + repoPath: string, +): Promise { + const resolution = await resolveRemoteTrackingTarget(repoPath); + if (!resolution.ok) { return { ahead: 0, behind: 0, branch: null, - error: "Unable to determine remote tracking branch", + error: resolution.error, hasRemoteChanges: false, remote: null, repositoryId, - upstream, + upstream: resolution.upstream, }; } - try { - await exec( - ["fetch", "--prune", remote, `+refs/heads/${branch}:refs/remotes/${remote}/${branch}`], - repoPath, - ); - } catch (error) { - const message = error instanceof Error ? error.message : "Remote fetch failed"; + const { target } = resolution; + const fetchResult = await fetchRemoteTrackingTarget(repoPath, target); + if (!fetchResult.ok) { return { ahead: 0, behind: 0, - branch, - error: message, + branch: target.branch, + error: fetchResult.error, hasRemoteChanges: false, - remote, + remote: target.remote, repositoryId, - upstream, + upstream: target.upstream, }; } + const { branch, remote, upstream } = target; + try { const compareRef = `refs/remotes/${remote}/${branch}`; const result = await exec( diff --git a/tests/unit/status.test.ts b/tests/unit/status.test.ts index 4f37628..632db0d 100644 --- a/tests/unit/status.test.ts +++ b/tests/unit/status.test.ts @@ -144,6 +144,95 @@ describe("checkRepoStatus", () => { expect(status.error).toContain("arashi clone"); expect(status.files).toHaveLength(0); }); + + test("refreshes tracked remote before parsing branch status", async () => { + const callOrder: string[] = []; + + const status = await checkRepoStatus("repo-a", process.cwd(), { + dependencies: { + fetchRemoteTrackingTarget: async (_repoPath, target) => { + callOrder.push(`fetch:${target.remote}/${target.branch}`); + return { ok: true }; + }, + getFullGitStatus: async () => ({ error: null, output: "" }), + getGitStatus: async () => { + callOrder.push("status"); + return { error: null, output: "## main...origin/main [behind 2]" }; + }, + resolveRemoteTrackingTarget: async () => { + callOrder.push("resolve"); + return { + ok: true as const, + target: { + branch: "main", + remote: "origin", + upstream: "origin/main", + }, + }; + }, + }, + }); + + expect(callOrder).toEqual(["resolve", "fetch:origin/main", "status"]); + expect(status.branch.behind).toBe(2); + expect(status.error).toBeNull(); + expect(status.refreshWarning).toBeNull(); + }); + + test("skips remote refresh when no tracking target can be resolved", async () => { + let fetchCalled = false; + + const status = await checkRepoStatus("repo-a", process.cwd(), { + dependencies: { + fetchRemoteTrackingTarget: async () => { + fetchCalled = true; + return { ok: true }; + }, + getFullGitStatus: async () => ({ error: null, output: "" }), + getGitStatus: async () => ({ error: null, output: "## main" }), + resolveRemoteTrackingTarget: async () => ({ + error: "No remotes configured for repository", + ok: false as const, + upstream: null, + }), + }, + }); + + expect(fetchCalled).toBe(false); + expect(status.branch.localBranch).toBe("main"); + expect(status.error).toBeNull(); + expect(status.refreshWarning).toBeNull(); + }); + + test("preserves local status when remote refresh fails", async () => { + const status = await checkRepoStatus("repo-a", process.cwd(), { + dependencies: { + fetchRemoteTrackingTarget: async () => ({ + error: "Git command failed: authentication required", + ok: false as const, + }), + getFullGitStatus: async () => ({ error: null, output: "" }), + getGitStatus: async () => ({ + error: null, + output: `## main...origin/main + M src/file.ts`, + }), + resolveRemoteTrackingTarget: async () => ({ + ok: true as const, + target: { + branch: "main", + remote: "origin", + upstream: "origin/main", + }, + }), + }, + }); + + expect(status.error).toBeNull(); + expect(status.files).toHaveLength(1); + expect(status.refreshWarning).toContain("Remote tracking may be stale"); + expect(formatShortLine(status)).toContain("remote tracking stale"); + }); }); describe("formatShortLine", () => {