root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(dashboard): close 8 review findings on git-status + integration-branch setting

Browse Source 
Settings (data-loss):
  - Non-model project keys now use null-as-delete instead of dropping
    undefined via JSON.stringify, so clearing Integration branch (Use
    dropdown / auto-detect) actually clears the persisted value.

isIndexStale (false-positive AND false-negative):
  - Replaced the empty-worktree heuristic with a reflog-anchored check:
    stale iff refs/heads/<integration>@{1} exists, HEAD is descendant of
    it, and `git diff-index --cached <prevTip>` is empty.

Auto-sync attribution in collectRecentMergeAdvances:
  - Match auto-sync events by (taskId, newSha) instead of taskId-only;
    re-merged tasks no longer have older advances mislabeled with the
    newest outcome.
  - Compare worktreePath after realpathSync on both sides; macOS symlink
    paths no longer cause permanent "needs action" false positives.

Extended path no longer 500s:
  - Route wraps computeExtendedGitStatus in try/catch and falls back to
    basic status on failure. Inner `branch --show-current` wrapped too
    so detached HEAD / non-git rootDir doesn't throw.

Integration branch falls back to remote-only ref:
  - When refs/heads/<branch> is missing, use refs/remotes/origin/<branch>
    as the integration tip. New `integrationTipSource` field
    ("local"|"remote-only"|"missing") drives a UI badge.

Copy commit hash:
  - Short-SHA copy is the default and matches what's displayed; a
    separate "full" button copies the 40-char headSha. Previously the
    single button silently copied the full SHA when extended was on.

Detached HEAD:
  - isOnIntegrationBranch left undefined when currentBranch is empty so
    the UI doesn't render "(not on <integration>)" against a
    no-branch state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-05-23 17:10:59 -07:00
parent 2d2e5b809f
commit ef12df4363
5 changed files with 168 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
.changeset/git-status-and-settings-review-fixes.md Normal file
View File

@@ -0,0 +1,21 @@
---
"@fusion/dashboard": patch
---
fix(dashboard): close 8 review findings on extended Git Manager status + Integration branch setting
**Settings persistence (data-loss)** — the project-settings patch builder now applies null-as-delete to all non-model keys, matching the global-settings branch. Previously, clearing the Integration branch field (picking `(auto-detect)` or clicking `Use dropdown`) set `integrationBranch: undefined`, which `JSON.stringify` silently dropped — the server retained the stale explicit value and the operator could not un-pin the branch from the UI.
**`isIndexStale` was wrong both directions** — the heuristic (`diff --cached --name-only` non-empty AND `diff --name-only` empty) fired false-positive on benign `git add` and false-negative whenever the worktree had any unrelated edit. Replaced with a reflog-anchored check: stale iff `refs/heads/<integrationBranch>@{1}` exists, HEAD is a descendant of it, and `git diff-index --cached <prevTip>` is empty (i.e. the index exactly matches the pre-advance state).
**Auto-sync attribution** — two fixes to `collectRecentMergeAdvances` in `register-git-github.ts`:
- Auto-sync events are now matched by `(taskId, newSha)` instead of `taskId`-only. A task that produced multiple advances over time no longer has all its older entries mislabeled with the most-recent outcome.
- `worktreePath` comparison now runs both sides through `fs.realpathSync` first. On macOS the merger emits canonicalized paths (via `canonicalizePath` in `worktree-pool.ts`) while the route was called with the store's raw `rootDir`; symlinked project paths caused every advance to be marked `needsAction: true` indefinitely.
**Extended path no longer 500s on git failure** — the `?extended=1` branch wraps `computeExtendedGitStatus` in its own try/catch and falls back to the basic status shape on any unhandled failure. Previously an unguarded `git branch --show-current` throw escaped to the route's outer catch and returned HTTP 500, while the basic path returned 200 with the swallowed-failure shape — surface parity matters because the dashboard always passes `extended=1` and would otherwise render an error toast where it should render the degraded panel. Also wrapped the same call inside `computeExtendedGitStatus` so detached-HEAD / non-git states return an empty `currentBranch` instead of throwing.
**Integration branch falls back to `refs/remotes/origin/<branch>`** — when the configured branch exists only as a remote-tracking ref (e.g. operator set `integrationBranch: "release/v2"` without ever `git switch`-ing it locally), `integrationTipSha` now resolves to the origin tip instead of being null. A new `integrationTipSource: "local" | "remote-only" | "missing"` field tells the UI which side won; the Git Manager surfaces this with a `(remote-only — run git switch <branch> to track locally)` sub-text and a `no ref found` error state when both refs are missing.
**Copy commit hash shows two buttons** — the Copy button now copies `status.commit` (the short SHA actually displayed in the `<code>` element). A second Copy-full button surfaces `status.headSha` for git operations that need the 40-char SHA. Previously the single button silently copied the full SHA when extended was on, so what the user saw on screen was no longer what they pasted.
**Detached HEAD no longer shows misleading "(not on main)"** — `git branch --show-current` returns empty on detached HEAD; the route now leaves `isOnIntegrationBranch` as `undefined` (not `false`) in that case, and the UI's "(not on <branch>)" sub-text only renders when we know we're on a different branch — not when we're on no branch at all.

4
packages/dashboard/app/api/legacy.ts
View File

@@ -2584,6 +2584,10 @@ export interface GitStatus {
integrationBranchSource?: "settings" | "origin-head" | "fallback";
isOnIntegrationBranch?: boolean;
integrationTipSha?: string | null;
/** "local" = `refs/heads/<branch>` exists; "remote-only" = only
* `refs/remotes/origin/<branch>` exists and was used as fallback;
* "missing" = neither ref exists. */
integrationTipSource?: "local" | "remote-only" | "missing";
originIntegrationTipSha?: string | null;
aheadOfIntegration?: number;
behindIntegration?: number;

27
packages/dashboard/app/components/GitManagerModal.tsx
View File

@@ -1022,6 +1022,9 @@ function StatusPanel({
<span className="gm-status-value">
<GitBranchIcon size={14} />
<span>{status.branch}</span>
{/* Only flag "not on <integration>" when we know the worktree IS
on a branch — detached HEAD (isOnIntegrationBranch undefined)
is a non-branch state, not "on the wrong branch." */}
{status.integrationBranch && status.isOnIntegrationBranch === false && (
<span className="gm-status-sub" title="Currently on a non-integration branch">
{" "}(not on {status.integrationBranch})
@@ -1035,11 +1038,21 @@ function StatusPanel({
<code className="gm-hash">{status.commit}</code>
<button
className="gm-icon-btn"
onClick={() => copyToClipboard(status.headSha ?? status.commit, "commit hash")}
title="Copy commit hash"
onClick={() => copyToClipboard(status.commit, "commit hash")}
title={`Copy short commit hash${status.headSha ? " (use the full SHA below for git operations)" : ""}`}
>
<Copy size={12} />
</button>
{status.headSha && (
<button
className="gm-icon-btn"
onClick={() => copyToClipboard(status.headSha!, "full commit hash")}
title="Copy full 40-char SHA"
>
<Copy size={12} />
<span style={{ fontSize: 10, marginLeft: 2 }}>full</span>
</button>
)}
</span>
</div>
<div className="gm-status-card">
@@ -1113,6 +1126,16 @@ function StatusPanel({
{status.integrationTipSha && (
<span className="gm-status-sub">
tip <code className="gm-hash">{status.integrationTipSha.slice(0, 8)}</code>
{status.integrationTipSource === "remote-only" && (
<>
{" "}<span title="No local refs/heads/<branch>; using refs/remotes/origin/<branch> as the integration tip.">(remote-only — run <code>git switch {status.integrationBranch}</code> to track locally)</span>
</>
)}
</span>
)}
{status.integrationTipSource === "missing" && (
<span className="gm-status-sub gm-status-conflict" title="Neither refs/heads nor refs/remotes/origin has this branch">
no ref found for {status.integrationBranch}
</span>
)}
</div>

13
packages/dashboard/app/components/SettingsModal.tsx
View File

@@ -2065,8 +2065,17 @@ export function SettingsModal({
}
}
} else {
// For non-model settings: existing behavior
(projectPatch as Record<string, unknown>)[key] = value;
// For non-model settings: pass value through. Apply the same
// null-as-delete semantics as the global patch builder above so the
// user can actually CLEAR an explicit value (e.g. unpin
// `integrationBranch` back to auto-detect). Without this, setting
// the form value to `undefined` causes JSON.stringify to drop the
// key and the server retains the previous explicit value.
if (value === undefined && initialProjectValue !== undefined && initialProjectValue !== null) {
(projectPatch as Record<string, unknown>)[key] = null;
} else {
(projectPatch as Record<string, unknown>)[key] = value;
}
}
}

134
packages/dashboard/src/routes/register-git-github.ts
View File

@@ -1,5 +1,6 @@
import { type NextFunction, type Request, type Response } from "express";
import { isAbsolute, resolve, relative } from "node:path";
import { realpathSync } from "node:fs";
import { exec as execCb, spawn } from "node:child_process";
import { promisify } from "node:util";
import type {
@@ -400,6 +401,11 @@ export interface ExtendedGitStatus {
integrationBranchSource?: "settings" | "origin-head" | "fallback";
isOnIntegrationBranch?: boolean;
integrationTipSha?: string | null;
/** Where `integrationTipSha` was resolved from. `"local"` = the branch
* exists locally; `"remote-only"` = the branch only exists as
* `refs/remotes/origin/<branch>` and was used as a fallback; `"missing"` =
* neither ref exists, so the integration tip is null. */
integrationTipSource?: "local" | "remote-only" | "missing";
originIntegrationTipSha?: string | null;
aheadOfIntegration?: number;
behindIntegration?: number;
@@ -484,21 +490,35 @@ async function computeDirtyDetails(cwd: string): Promise<ExtendedGitStatus["dirt
}
}
async function isIndexStale(cwd: string): Promise<boolean | undefined> {
// The FN-INDEX-DESYNC scenario: the merger advanced refs/heads/<branch>
async function isIndexStale(cwd: string, integrationBranch: string): Promise<boolean | undefined> {
// The FN-INDEX-DESYNC scenario: the merger advanced refs/heads/<integration>
// locally so HEAD points at the new tip, but the index still reflects the
// previous tip. Detect by diffing the index tree against HEAD's tree —
// when the worktree is "clean" relative to HEAD (i.e. all `git status`
// changes are really inverted-index artifacts), this returns true.
// *previous* tip. Detect precisely by comparing the index against
// `refs/heads/<integration>@{1}` (the reflog entry for the tip BEFORE the
// advance) — if `git diff-index --cached <prevTip>` is empty, the index
// exactly matches the pre-advance state, and HEAD must be a descendant of
// that prev tip for the signal to be the merger's doing rather than a
// hand-staged reset.
//
// The earlier heuristic (`diff --cached --name-only` non-empty AND
// `diff --name-only` empty) fired both false-positive on legitimate
// `git add` work and false-negative when the user had any unrelated
// worktree edit. The reflog-anchored check is unambiguous.
try {
const idx = (await runGitCommand(["diff", "--cached", "--name-only"], cwd, 5_000)).trim();
if (idx.length === 0) return false;
const wt = (await runGitCommand(["diff", "--name-only"], cwd, 5_000)).trim();
// Index disagrees with HEAD but worktree matches index — that's the
// tell-tale stale-index signal. The merger's auto-sync hook would
// normally clear it; surfacing it lets operators see when sync was off
// or had to skip.
return idx.length > 0 && wt.length === 0;
const prevTip = await revParse(cwd, `refs/heads/${integrationBranch}@{1}`);
if (!prevTip) return false;
const headSha = await revParse(cwd, "HEAD");
if (!headSha || headSha === prevTip) return false;
let isDescendant = false;
try {
await runGitCommand(["merge-base", "--is-ancestor", prevTip, "HEAD"], cwd, 5_000);
isDescendant = true;
} catch {
isDescendant = false;
}
if (!isDescendant) return false;
const diffOut = (await runGitCommand(["diff-index", "--cached", "--name-only", prevTip], cwd, 5_000)).trim();
return diffOut.length === 0;
} catch {
return undefined;
}
@@ -514,6 +534,20 @@ async function computeStashCount(cwd: string): Promise<number | undefined> {
}
}
/** Canonicalize a filesystem path for cross-process equality checks. The
* merger emits audit events with `worktreePath` run through `realpath` (via
* `canonicalizePath` in worktree-pool.ts); the route is called with the
* store's raw `rootDir`. On macOS the two routinely differ through
* `/private` symlinks. Resolving both ends through `realpathSync` (with a
* graceful fallback if the path no longer exists) gives a stable key. */
function canonicalForCompare(p: string): string {
try {
return realpathSync(p);
} catch {
return p;
}
}
async function collectRecentMergeAdvances(
scopedStore: TaskStore & {
getRunAuditEvents?: (filters: {
@@ -531,18 +565,31 @@ async function collectRecentMergeAdvances(
mutationType: "merge:integration-ref-advance",
limit: 10,
});
const autoSyncByTask = new Map<string, string>();
// Key by (taskId, newSha) so each specific advance is paired with its own
// outcome — the previous map-by-taskId scheme paired older advances with
// the most-recent task's outcome whenever a task generated multiple
// advances over time. Auto-sync events store the destination tip in
// `metadata.newSha` (set by the merger's runMergeAdvanceAutoSync hook).
const wantPath = canonicalForCompare(worktreePath);
const autoSyncByAdvance = new Map<string, string>();
const pairKey = (tid: string, toSha: string) => `${tid}:${toSha}`;
for (const ev of scopedStore.getRunAuditEvents({
domain: "git",
mutationType: "merge:auto-sync",
limit: 50,
limit: 200,
})) {
const md = ev.metadata as { worktreePath?: unknown; outcome?: unknown; taskId?: unknown } | undefined;
const md = ev.metadata as { worktreePath?: unknown; outcome?: unknown; taskId?: unknown; newSha?: unknown } | undefined;
if (!md || typeof md !== "object") continue;
if (typeof md.worktreePath !== "string" || md.worktreePath !== worktreePath) continue;
if (typeof md.worktreePath !== "string") continue;
if (canonicalForCompare(md.worktreePath) !== wantPath) continue;
if (typeof md.outcome !== "string") continue;
if (typeof md.newSha !== "string") continue;
const tid = typeof md.taskId === "string" ? md.taskId : (typeof ev.taskId === "string" ? ev.taskId : "");
if (tid && !autoSyncByTask.has(tid)) autoSyncByTask.set(tid, md.outcome);
if (!tid) continue;
const key = pairKey(tid, md.newSha);
// Events are timestamp DESC; the first occurrence of (tid, newSha) is the
// freshest outcome for that specific advance — keep it.
if (!autoSyncByAdvance.has(key)) autoSyncByAdvance.set(key, md.outcome);
}
const successOutcomes = new Set(["clean-sync", "synced-with-edits-restored"]);
const out: NonNullable<ExtendedGitStatus["recentMergeAdvances"]> = [];
@@ -553,7 +600,7 @@ async function collectRecentMergeAdvances(
if (md.succeeded === false) continue;
const tid = typeof ev.taskId === "string" ? ev.taskId : "";
if (!tid) continue;
const autoSyncOutcome = autoSyncByTask.get(tid);
const autoSyncOutcome = autoSyncByAdvance.get(pairKey(tid, md.toSha));
out.push({
taskId: tid,
fromSha: typeof md.fromSha === "string" ? md.fromSha : null,
@@ -573,11 +620,29 @@ export async function computeExtendedGitStatus(rootDir: string, scopedStore: Tas
rootDir,
settings as { integrationBranch?: unknown; baseBranch?: unknown } | null,
);
const currentBranch = (await runGitCommand(["branch", "--show-current"], rootDir, 5_000)).trim();
const isOnIntegrationBranch = currentBranch === integrationBranch;
let currentBranch = "";
try {
currentBranch = (await runGitCommand(["branch", "--show-current"], rootDir, 5_000)).trim();
} catch {
// Detached HEAD / non-git rootDir / corrupted state — leave currentBranch
// empty and let isOnIntegrationBranch resolve to undefined below so the
// UI doesn't render a misleading "(not on <branch>)" sub-text.
}
// `git branch --show-current` returns empty on detached HEAD; treat that as
// "unknown" rather than "definitely not on the integration branch" so the
// UI can render an honest detached-HEAD state instead of "(not on main)".
const isOnIntegrationBranch = currentBranch.length === 0 ? undefined : currentBranch === integrationBranch;
const headSha = (await revParse(rootDir, "HEAD")) ?? undefined;
const integrationTipSha = await revParse(rootDir, `refs/heads/${integrationBranch}`);
// Prefer the local head; fall back to the remote-tracking ref so projects
// whose `integrationBranch` setting names a branch that exists only on
// origin (e.g. `release/v2` the operator has never `git switch`-ed
// locally) still get a meaningful tip + ahead/behind comparison instead of
// a silently-empty integration card.
const localIntegrationTip = await revParse(rootDir, `refs/heads/${integrationBranch}`);
const originIntegrationTipSha = await revParse(rootDir, `refs/remotes/origin/${integrationBranch}`);
const integrationTipSha = localIntegrationTip ?? originIntegrationTipSha ?? null;
const integrationTipSource: ExtendedGitStatus["integrationTipSource"] =
localIntegrationTip ? "local" : originIntegrationTipSha ? "remote-only" : "missing";
let aheadOfIntegration: number | undefined;
let behindIntegration: number | undefined;
@@ -587,14 +652,14 @@ export async function computeExtendedGitStatus(rootDir: string, scopedStore: Tas
}
let aheadOfOriginIntegration: number | undefined;
let behindOriginIntegration: number | undefined;
if (originIntegrationTipSha && integrationTipSha) {
const ab = await aheadBehind(rootDir, integrationTipSha, originIntegrationTipSha);
if (originIntegrationTipSha && localIntegrationTip) {
const ab = await aheadBehind(rootDir, localIntegrationTip, originIntegrationTipSha);
if (ab) { aheadOfOriginIntegration = ab.ahead; behindOriginIntegration = ab.behind; }
}
const [dirtyDetails, indexStaleVsHead, stashCount, recentMergeAdvances] = await Promise.all([
computeDirtyDetails(rootDir),
isIndexStale(rootDir),
isIndexStale(rootDir, integrationBranch),
computeStashCount(rootDir),
collectRecentMergeAdvances(
scopedStore as TaskStore & {
@@ -610,6 +675,7 @@ export async function computeExtendedGitStatus(rootDir: string, scopedStore: Tas
integrationBranchSource,
isOnIntegrationBranch,
integrationTipSha,
integrationTipSource,
originIntegrationTipSha,
aheadOfIntegration,
behindIntegration,
@@ -2422,8 +2488,22 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
res.json(status);
return;
}
const extended = await computeExtendedGitStatus(rootDir, scopedStore);
res.json({ ...status, ...extended });
// Compute extended status best-effort: if any unhandled git or store
// failure escapes the helpers (timeout on `branch --show-current`,
// missing reflog, store layer throws), degrade to the basic shape
// rather than returning HTTP 500. The basic path swallows the same
// failures via getGitStatus's broad try/catch — surface parity matters
// because the dashboard always passes ?extended=1 and would otherwise
// render an error toast where the legacy path would render a degraded
// but usable panel.
try {
const extended = await computeExtendedGitStatus(rootDir, scopedStore);
res.json({ ...status, ...extended });
} catch (extErr: unknown) {
const message = extErr instanceof Error ? extErr.message : String(extErr);
console.warn(`[git-status] extended computation failed; returning basic status: ${message}`);
res.json(status);
}
} catch (err: unknown) {
if (err instanceof ApiError) {
throw err;