refactor(server): tighten git helpers and inventory shape

- isSafeGitUpstreamToken: allowlist regex over per-char loop
- gitStatusSnapshotAsync: one `status -b` instead of two subprocesses;
  derive short output from the same payload
- collectInventoryEntry: extract buildEntry helper, dedupe four return
  paths, drop ~40 lines
- upstreamNoteFor: take hasCounts boolean, signature no longer lies
- getPresetEntry: return discriminated {ok} union to match
  PresetLoadResult convention used elsewhere in the file
- pathMatchesWorkspaceRootHint: drop redundant pre-normalized branches
- resolveWorkspaceRoots: hoist cwd fallback
- git-inventory-tool: reuse makeSkipEntry for not_a_git_repository
  branch; drop dead String().trim() noise after non-empty guard

Behavior preserved; halves the subprocess count on the status path.

Author: AlbinoGeek

src/server/git-inventory-tool.ts

@@ -81,9 +81,7 @@ export function registerGitInventoryTool(server: FastMCP): void {
       }
       const useFixed = hasRemote && hasBranch;
       if (useFixed) {
-        const r = String(fixedRemote).trim();
-        const b = String(fixedBranch).trim();
-        if (!isSafeGitUpstreamToken(r) || !isSafeGitUpstreamToken(b)) {
+        if (!isSafeGitUpstreamToken(fixedRemote!.trim()) || !isSafeGitUpstreamToken(fixedBranch!.trim())) {
           return jsonRespond({
             error: "invalid_remote_or_branch",
             message:
@@ -113,22 +111,7 @@ export function registerGitInventoryTool(server: FastMCP): void {
                 remote: fixedRemote,
                 branch: fixedBranch,
               },
-              entries: [
-                {
-                  label: workspaceRoot,
-                  path: workspaceRoot,
-                  branchStatus: "",
-                  shortStatus: "",
-                  detached: false,
-                  headAbbrev: "",
-                  upstreamMode: useFixed ? "fixed" : "auto",
-                  upstreamRef: null,
-                  ahead: null,
-                  behind: null,
-                  upstreamNote: "",
-                  skipReason: JSON.stringify(err),
-                },
-              ],
+              entries: [makeSkipEntry(workspaceRoot, workspaceRoot, useFixed ? "fixed" : "auto", JSON.stringify(err))],
             });
           } else {
             mdChunks.push(`# Git inventory`, "", jsonRespond(err), "");

src/server/git.ts

@@ -94,17 +94,7 @@ export function isSafeGitUpstreamToken(s: string): boolean {
   const t = s.trim();
   if (t.length === 0 || t.length > 256) return false;
   if (t.includes("..")) return false;
-  if (t.startsWith("-")) return false;
-  if (t.includes("@")) return false;
-  const forbidden = new Set("{}[]~^:?*\\".split(""));
-  for (let i = 0; i < t.length; i++) {
-    const ch = t.charAt(i);
-    const c = ch.charCodeAt(0);
-    if (c < 32 || c === 127) return false;
-    if (/\s/.test(ch)) return false;
-    if (forbidden.has(ch)) return false;
-  }
-  return true;
+  return /^(?!-)[A-Za-z0-9_./+-]+$/.test(t);
 }
 
 // ---------------------------------------------------------------------------
@@ -122,9 +112,7 @@ export async function asyncPool<T, R>(
     for (;;) {
       const i = next++;
       if (i >= items.length) break;
-      const item = items[i];
-      if (item === undefined) break;
-      results[i] = await fn(item);
+      results[i] = await fn(items[i]!);
     }
   }
   const n = Math.min(Math.max(1, concurrency), Math.max(1, items.length));
@@ -163,16 +151,16 @@ export async function gitStatusSnapshotAsync(cwd: string): Promise<{
   branchOk: boolean;
   shortOk: boolean;
 }> {
-  const [br, sh] = await Promise.all([
-    spawnGitAsync(cwd, ["status", "--short", "-b"]),
-    spawnGitAsync(cwd, ["status", "--short"]),
-  ]);
-  return {
-    branchOk: br.ok,
-    shortOk: sh.ok,
-    branchLine: br.ok ? br.stdout.trimEnd() : gitStatusFailText(br),
-    shortLine: sh.ok ? sh.stdout.trimEnd() : gitStatusFailText(sh),
-  };
+  const r = await spawnGitAsync(cwd, ["status", "--short", "-b"]);
+  if (!r.ok) {
+    const text = gitStatusFailText(r);
+    return { branchOk: false, shortOk: false, branchLine: text, shortLine: text };
+  }
+  const full = r.stdout.trimEnd();
+  const nl = full.indexOf("\n");
+  const branchLine = full;
+  const shortLine = nl >= 0 ? full.slice(nl + 1) : "";
+  return { branchOk: true, shortOk: true, branchLine, shortLine };
 }
 
 export async function gitStatusShortBranchAsync(

src/server/inventory.ts

@@ -68,10 +68,36 @@ export function buildInventorySectionMarkdown(e: InventoryEntryJson): string[] {
   return [`## ${e.label}`, `path: ${e.path}`, "```text", lines.join("\n"), "```", ``];
 }
 
-function upstreamNoteFor(ref: string, ahead: string | null, behind: string | null): string {
-  return ahead != null && behind != null
-    ? `tracking ${ref}`
-    : `upstream ${ref} (counts unreadable)`;
+function upstreamNoteFor(ref: string, hasCounts: boolean): string {
+  return hasCounts ? `tracking ${ref}` : `upstream ${ref} (counts unreadable)`;
+}
+
+function buildEntry(params: {
+  label: string;
+  absPath: string;
+  branchStatus: string;
+  shortStatus: string;
+  detached: boolean;
+  headAbbrev: string;
+  upstreamMode: "auto" | "fixed";
+  upstreamRef: string | null;
+  ahead: string | null;
+  behind: string | null;
+  upstreamNote: string;
+}): InventoryEntryJson {
+  return {
+    label: params.label,
+    path: params.absPath,
+    branchStatus: params.branchStatus,
+    shortStatus: params.shortStatus,
+    detached: params.detached,
+    headAbbrev: params.headAbbrev || "(unknown)",
+    upstreamMode: params.upstreamMode,
+    upstreamRef: params.upstreamRef,
+    ahead: params.ahead,
+    behind: params.behind,
+    upstreamNote: params.upstreamNote,
+  };
 }
 
 export async function collectInventoryEntry(
@@ -89,81 +115,53 @@ export async function collectInventoryEntry(
   const shortStatus = snap.shortLine;
   const headAbbrev = headR.ok ? headR.stdout.trim() : "";
   const detached = !headR.ok || headAbbrev === "HEAD" || headAbbrev.endsWith("/HEAD");
+  const base = { label, absPath, branchStatus, shortStatus, detached, headAbbrev };
 
-  const useFixed = fixedRemote !== undefined && fixedBranch !== undefined;
-
-  if (useFixed) {
-    const remote = fixedRemote;
-    const branch = fixedBranch;
-    const verify = await spawnGitAsync(absPath, ["rev-parse", "--verify", `${remote}/${branch}`]);
+  if (fixedRemote !== undefined && fixedBranch !== undefined) {
+    const ref = `${fixedRemote}/${fixedBranch}`;
+    const verify = await spawnGitAsync(absPath, ["rev-parse", "--verify", ref]);
     if (!verify.ok) {
-      return {
-        label,
-        path: absPath,
-        branchStatus,
-        shortStatus,
-        detached,
-        headAbbrev: headAbbrev || "(unknown)",
+      return buildEntry({
+        ...base,
         upstreamMode: "fixed",
-        upstreamRef: `${remote}/${branch}`,
+        upstreamRef: ref,
         ahead: null,
         behind: null,
-        upstreamNote: `(no local ref ${remote}/${branch} or unreadable)`,
-      };
+        upstreamNote: `(no local ref ${ref} or unreadable)`,
+      });
     }
-    const ref = `${remote}/${branch}`;
     const { ahead, behind } = await fetchAheadBehind(absPath, ref);
-    return {
-      label,
-      path: absPath,
-      branchStatus,
-      shortStatus,
-      detached,
-      headAbbrev: headAbbrev || "(unknown)",
+    return buildEntry({
+      ...base,
       upstreamMode: "fixed",
       upstreamRef: ref,
       ahead,
       behind,
-      upstreamNote: upstreamNoteFor(ref, ahead, behind),
-    };
+      upstreamNote: upstreamNoteFor(ref, ahead != null && behind != null),
+    });
   }
 
   const upVerify = await spawnGitAsync(absPath, ["rev-parse", "--verify", "@{u}"]);
   if (!upVerify.ok) {
-    let note = "no upstream configured";
-    if (detached) {
-      note = "detached HEAD — no upstream";
-    }
-    return {
-      label,
-      path: absPath,
-      branchStatus,
-      shortStatus,
-      detached,
-      headAbbrev: headAbbrev || "(unknown)",
+    return buildEntry({
+      ...base,
       upstreamMode: "auto",
       upstreamRef: null,
       ahead: null,
       behind: null,
-      upstreamNote: note,
-    };
+      upstreamNote: detached ? "detached HEAD — no upstream" : "no upstream configured",
+    });
   }
 
   const abbrevR = await spawnGitAsync(absPath, ["rev-parse", "--abbrev-ref", "@{u}"]);
   const upstreamRef = abbrevR.ok ? abbrevR.stdout.trim() : "@{u}";
   const { ahead, behind } = await fetchAheadBehind(absPath, "@{u}");
-
-  return {
-    label,
-    path: absPath,
-    branchStatus,
-    shortStatus,
-    detached,
-    headAbbrev: headAbbrev || "(unknown)",
+  return buildEntry({
+    ...base,
     upstreamMode: "auto",
     upstreamRef,
     ahead,
     behind,
-    upstreamNote: upstreamNoteFor(upstreamRef, ahead, behind),
-  };
+    upstreamNote: upstreamNoteFor(upstreamRef, ahead != null && behind != null),
+  });
 }

src/server/presets.ts

@@ -124,11 +124,12 @@ export function presetLoadErrorPayload(
 export function getPresetEntry(
   gitTop: string,
   presetName: string,
-): { entry: PresetEntry; presetSchemaVersion?: string } | { error: Record<string, unknown> } {
+): { ok: true; entry: PresetEntry; presetSchemaVersion?: string } | { ok: false; error: Record<string, unknown> } {
   const loaded = loadPresetsFromGitTop(gitTop);
   if (!loaded.ok) {
     if (loaded.reason === "missing") {
       return {
+        ok: false,
         error: {
           error: "preset_not_found",
           preset: presetName,
@@ -137,19 +138,20 @@ export function getPresetEntry(
         },
       };
     }
-    return { error: presetLoadErrorPayload(gitTop, loaded) };
+    return { ok: false, error: presetLoadErrorPayload(gitTop, loaded) };
   }
   const entry = loaded.data[presetName];
   if (!entry) {
     return {
+      ok: false,
       error: {
         error: "preset_not_found",
         preset: presetName,
         presetFile: join(gitTop, PRESET_FILE_PATH),
       },
     };
   }
-  return { entry, presetSchemaVersion: loaded.schemaVersion };
+  return { ok: true, entry, presetSchemaVersion: loaded.schemaVersion };
 }
 
 export function mergeNestedRoots(
@@ -191,7 +193,7 @@ export function applyPresetNestedRoots(
   | { ok: true; nestedRoots: string[] | undefined; presetSchemaVersion?: string }
   | { ok: false; error: Record<string, unknown> } {
   const got = getPresetEntry(gitTop, presetName);
-  if ("error" in got) return { ok: false, error: got.error };
+  if (!got.ok) return { ok: false, error: got.error };
   const fromPreset = got.entry.nestedRoots;
   let nestedRoots: string[] | undefined = inlineNestedRoots;
   if (presetMerge) {
@@ -211,7 +213,7 @@ export function applyPresetParityPairs(
   | { ok: true; pairs: ParityPair[] | undefined; presetSchemaVersion?: string }
   | { ok: false; error: Record<string, unknown> } {
   const got = getPresetEntry(gitTop, presetName);
-  if ("error" in got) return { ok: false, error: got.error };
+  if (!got.ok) return { ok: false, error: got.error };
   const fromPreset = got.entry.parityPairs;
   let pairs: ParityPair[] | undefined = inlinePairs;
   if (presetMerge) {

src/server/roots.ts

@@ -1,4 +1,4 @@
-import { basename, isAbsolute, resolve } from "node:path";
+import { basename, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
 
 import type { FastMCP } from "fastmcp";
@@ -30,19 +30,13 @@ export function listFileRoots(server: FastMCP): string[] {
 export function pathMatchesWorkspaceRootHint(rootPath: string, hint: string): boolean {
   const h = hint.trim();
   if (!h) return true;
-  if (basename(rootPath) === h) return true;
   const absRoot = resolve(rootPath);
-  if (absRoot === h) return true;
-  try {
-    if (isAbsolute(h) && resolve(h) === absRoot) return true;
-  } catch {
-    /* invalid absolute hint */
-  }
   const normRoot = absRoot.replace(/\\/g, "/").replace(/\/+$/, "");
   const normHint = h.replace(/\\/g, "/").replace(/\/+$/, "").replace(/^\/+/, "");
   if (!normHint) return true;
   if (normRoot === normHint) return true;
-  return normRoot.endsWith(`/${normHint}`);
+  if (normRoot.endsWith(`/${normHint}`)) return true;
+  return basename(rootPath) === h;
 }
 
 export type RootPick = {
@@ -60,31 +54,19 @@ export function resolveWorkspaceRoots(server: FastMCP, args: RootPick): ResolveR
     return { ok: true, roots: [resolve(args.workspaceRoot.trim())] };
   }
   const fileRoots = listFileRoots(server);
+  const fallback: ResolveRootsResult = { ok: true, roots: [process.cwd()] };
   if (args.allWorkspaceRoots) {
-    if (fileRoots.length === 0) {
-      return { ok: true, roots: [process.cwd()] };
-    }
-    return { ok: true, roots: fileRoots };
+    return fileRoots.length === 0 ? fallback : { ok: true, roots: fileRoots };
   }
   if (args.rootIndex != null) {
     const r = fileRoots[args.rootIndex];
     if (!r) {
-      return {
-        ok: false,
-        error: {
-          error: "root_index_out_of_range",
-          rootIndex: args.rootIndex,
-          rootCount: fileRoots.length,
-        },
-      };
+      return { ok: false, error: { error: "root_index_out_of_range", rootIndex: args.rootIndex, rootCount: fileRoots.length } };
     }
     return { ok: true, roots: [r] };
   }
   const primary = fileRoots[0];
-  if (primary !== undefined) {
-    return { ok: true, roots: [primary] };
-  }
-  return { ok: true, roots: [process.cwd()] };
+  return primary !== undefined ? { ok: true, roots: [primary] } : fallback;
 }
 
 /**