fix(open-knowledge): discover git identity from included git configs (#2864)

.changeset/git-identity-included-configs.md

@@ -0,0 +1,5 @@
+---
+"@inkeep/open-knowledge": patch
+---
+
+Discover git committer identity supplied through included git configs. Auto-save commit identity resolution now reads the effective merged git config (`git config --get`) instead of probing the `--worktree`/`--local`/`--global` scopes one at a time. A scope-limited read only sees values written literally in that one file, so an identity provided via an `include` or `includeIf` directive (for example `gitdir:` or `hasconfig:remote.*.url:` identity switching) was invisible and OK fell back to prompting. The merged read is exactly what git resolves for a commit, so it honors full scope precedence and resolves included configs.

packages/app/src/hooks/use-git-sync-status.ts

@@ -44,7 +44,7 @@ export interface GitSyncStatus {
   /** User's sync toggle preference (false by default — disabled for safety). */
   syncEnabled: boolean;
   /**
-   * Soft signal: the git identity chain (local → global → OAuth)
+   * Soft signal: the git identity chain (merged git config → OAuth)
    * returned null on the last probe. Commits still succeed under a default
    * identity — the UI surfaces a non-blocking nudge to set a real one.
    */

packages/server/src/git-identity.includes.test.ts

@@ -0,0 +1,102 @@
+/**
+ * Integration tests for resolveGitIdentity against a real git repo whose
+ * identity is supplied through `include` / `includeIf` directives.
+ *
+ * Regression: users who switch committer identity via
+ * `includeIf "gitdir:…"` or `includeIf "hasconfig:remote.*.url:…"` in their
+ * global config saw OK fail to discover any identity. The old chain probed
+ * `--worktree` / `--local` / `--global` scopes one at a time, and a
+ * scope-limited read never resolves included config — only the effective
+ * merged read (`git config --get`) does.
+ *
+ * Isolation: GIT_CONFIG_GLOBAL points git at a temp global file and
+ * GIT_CONFIG_SYSTEM is neutralized, so the ambient developer / CI git identity
+ * cannot leak in. All other GIT_* vars are scrubbed so an inherited GIT_DIR
+ * doesn't redirect the reads into the surrounding repo.
+ */
+
+import { spawnSync } from 'node:child_process';
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { basename, join } from 'node:path';
+import { afterEach, beforeEach, describe, expect, test } from 'vitest';
+
+import { resolveGitIdentity } from './git-identity.ts';
+
+function run(cwd: string, ...args: string[]): { status: number; stderr: string } {
+  const result = spawnSync('git', args, { cwd, encoding: 'utf-8', timeout: 10_000 });
+  return { status: result.status ?? -1, stderr: result.stderr?.trim() ?? '' };
+}
+
+describe('resolveGitIdentity with included git config', () => {
+  let tmp: string;
+  let repo: string;
+  let identityFile: string;
+  let globalFile: string;
+  let savedGitEnv: Record<string, string | undefined>;
+
+  beforeEach(() => {
+    // Scrub inherited GIT_* so a parent shell can't redirect our reads, then
+    // pin the global/system config so the developer's real identity can't leak.
+    savedGitEnv = {};
+    for (const k of Object.keys(process.env)) {
+      if (k.startsWith('GIT_')) {
+        savedGitEnv[k] = process.env[k];
+        delete process.env[k];
+      }
+    }
+
+    tmp = mkdtempSync(join(tmpdir(), 'ok-git-identity-inc-'));
+    repo = join(tmp, 'repo');
+    identityFile = join(tmp, 'identity');
+    globalFile = join(tmp, 'gitconfig');
+
+    process.env.GIT_CONFIG_GLOBAL = globalFile;
+    process.env.GIT_CONFIG_NOSYSTEM = '1';
+
+    writeFileSync(identityFile, '[user]\n\tname = Included Dev\n\temail = included@example.com\n');
+
+    expect(run(tmp, 'init', '-b', 'main', 'repo').status).toBe(0);
+  });
+
+  afterEach(() => {
+    rmSync(tmp, { recursive: true, force: true });
+    for (const k of Object.keys(process.env)) {
+      if (k.startsWith('GIT_')) delete process.env[k];
+    }
+    for (const [k, v] of Object.entries(savedGitEnv)) {
+      if (v === undefined) delete process.env[k];
+      else process.env[k] = v;
+    }
+  });
+
+  test('resolves identity from an unconditional [include] directive', async () => {
+    writeFileSync(globalFile, `[include]\n\tpath = ${identityFile}\n`);
+    const resolved = await resolveGitIdentity(repo);
+    expect(resolved).toEqual({ name: 'Included Dev', email: 'included@example.com' });
+  });
+
+  test('resolves identity from includeIf "gitdir:" matching the repo', async () => {
+    // Glob on the unique temp-dir basename so symlinked temp roots
+    // (e.g. macOS /var -> /private/var) still match.
+    const marker = basename(tmp);
+    writeFileSync(globalFile, `[includeIf "gitdir:**/${marker}/**"]\n\tpath = ${identityFile}\n`);
+    const resolved = await resolveGitIdentity(repo);
+    expect(resolved).toEqual({ name: 'Included Dev', email: 'included@example.com' });
+  });
+
+  test('resolves identity from includeIf "hasconfig:remote.*.url:" once the remote matches', async () => {
+    writeFileSync(
+      globalFile,
+      '[includeIf "hasconfig:remote.*.url:git@github.com:acme/**"]\n' +
+        `\tpath = ${identityFile}\n`,
+    );
+
+    // Before the matching remote exists the condition is false → no identity.
+    expect(await resolveGitIdentity(repo)).toBeNull();
+
+    expect(run(repo, 'remote', 'add', 'origin', 'git@github.com:acme/thing.git').status).toBe(0);
+    const resolved = await resolveGitIdentity(repo);
+    expect(resolved).toEqual({ name: 'Included Dev', email: 'included@example.com' });
+  });
+});

packages/server/src/git-identity.test.ts

@@ -2,7 +2,12 @@
  * Unit tests for resolveGitIdentity() — chain order + fallback logic.
  * Uses injected GitConfigReader so no actual git or simple-git calls are made.
  *
- * Chain: per-worktree → repo-local → global → tokenStore → null
+ * Chain: effective merged git config → tokenStore → null
+ *
+ * Scope precedence (system < global < local < worktree) and include / includeIf
+ * resolution are delegated to git's own merged read; that behavior is covered by
+ * the real-git integration tests in git-identity.worktree.test.ts and
+ * git-identity.includes.test.ts.
  */
 
 import { describe, expect, test } from 'vitest';
@@ -14,11 +19,9 @@ import {
 
 // ─── Helpers ──────────────────────────────────────────────────────────────────
 
-/** Build a mock GitConfigReader that returns pre-defined values per (key, scope). */
-function mockReader(
-  values: Partial<Record<string, Partial<Record<'worktree' | 'local' | 'global', string | null>>>>,
-): GitConfigReader {
-  return (_dir, key, scope) => values[key]?.[scope] ?? null;
+/** Build a mock GitConfigReader that returns pre-defined effective values per key. */
+function mockReader(values: Partial<Record<string, string | null>>): GitConfigReader {
+  return (_dir, key) => values[key] ?? null;
 }
 
 /** A minimal TokenStore stub. */
@@ -32,78 +35,39 @@ function makeTokenStore(entry: { login: string; name?: string; email?: string }
 // ─── Chain order tests ────────────────────────────────────────────────────────
 
 describe('resolveGitIdentity chain order', () => {
-  test('Step 1 (worktree): per-worktree identity wins over local + global', async () => {
-    const reader = mockReader({
-      'user.name': { worktree: 'WT Dev', local: 'Local Dev', global: 'Global Dev' },
-      'user.email': {
-        worktree: 'wt@example.com',
-        local: 'local@example.com',
-        global: 'global@example.com',
-      },
-    });
-    const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'WT Dev', email: 'wt@example.com' });
-  });
-
-  test('Step 1 partial: worktree name only — falls through to local pair', async () => {
-    const reader = mockReader({
-      'user.name': { worktree: 'WT Dev', local: 'Local Dev', global: 'Global Dev' },
-      'user.email': { worktree: null, local: 'local@example.com', global: 'global@example.com' },
-    });
-    const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Local Dev', email: 'local@example.com' });
-  });
-
-  test('Step 1 partial: worktree email only — falls through to local pair', async () => {
-    const reader = mockReader({
-      'user.name': { worktree: null, local: 'Local Dev', global: 'Global Dev' },
-      'user.email': {
-        worktree: 'wt@example.com',
-        local: 'local@example.com',
-        global: 'global@example.com',
-      },
-    });
-    const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Local Dev', email: 'local@example.com' });
-  });
-
-  test('Step 1 absent: empty worktree scope falls through to local (extension off / non-worktree)', async () => {
+  test('Step 1: returns the effective git config identity when name + email are both set', async () => {
     const reader = mockReader({
-      'user.name': { local: 'Local Dev' },
-      'user.email': { local: 'local@example.com' },
+      'user.name': 'Config Dev',
+      'user.email': 'config@example.com',
     });
     const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Local Dev', email: 'local@example.com' });
+    expect(result).toEqual({ name: 'Config Dev', email: 'config@example.com' });
   });
 
-  test('Step 2: returns repo-local identity when both name + email are set locally', async () => {
-    const reader = mockReader({
-      'user.name': { local: 'Local Dev', global: 'Global Dev' },
-      'user.email': { local: 'local@example.com', global: 'global@example.com' },
+  test('Step 1 partial: name only — falls through to token store', async () => {
+    const reader = mockReader({ 'user.name': 'Config Dev' });
+    const store = makeTokenStore({
+      login: 'octocat',
+      name: 'The Octocat',
+      email: 'cat@github.com',
     });
-    const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Local Dev', email: 'local@example.com' });
+    const result = await resolveGitIdentity('/fake/repo', store, 'github.com', reader);
+    expect(result).toEqual({ name: 'The Octocat', email: 'cat@github.com' });
   });
 
-  test('Step 2 partial: local name only — falls through to global', async () => {
-    const reader = mockReader({
-      'user.name': { local: 'Local Dev', global: 'Global Dev' },
-      'user.email': { local: null, global: 'global@example.com' },
-    });
+  test('Step 1 partial: email only — falls through to null when no token store', async () => {
+    const reader = mockReader({ 'user.email': 'config@example.com' });
     const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Global Dev', email: 'global@example.com' });
+    expect(result).toBeNull();
   });
 
-  test('Step 3: returns global identity when local is empty', async () => {
-    const reader = mockReader({
-      'user.name': { local: null, global: 'Global Dev' },
-      'user.email': { local: null, global: 'global@example.com' },
-    });
+  test('Step 1 partial: name only — falls through to null when no token store', async () => {
+    const reader = mockReader({ 'user.name': 'Config Dev' });
     const result = await resolveGitIdentity('/fake/repo', null, null, reader);
-    expect(result).toEqual({ name: 'Global Dev', email: 'global@example.com' });
+    expect(result).toBeNull();
   });
 
-  test('Step 4: uses tokenStore when both git configs are empty', async () => {
+  test('Step 2: uses tokenStore when git config is empty', async () => {
     const reader = mockReader({});
     const store = makeTokenStore({
       login: 'octocat',
@@ -114,14 +78,14 @@ describe('resolveGitIdentity chain order', () => {
     expect(result).toEqual({ name: 'The Octocat', email: 'cat@github.com' });
   });
 
-  test('Step 4: uses login as name fallback when entry.name is absent', async () => {
+  test('Step 2: uses login as name fallback when entry.name is absent', async () => {
     const reader = mockReader({});
     const store = makeTokenStore({ login: 'octocat', email: 'cat@github.com' });
     const result = await resolveGitIdentity('/fake/repo', store, 'github.com', reader);
     expect(result).toEqual({ name: 'octocat', email: 'cat@github.com' });
   });
 
-  test('Step 4: synthesizes noreply email when entry.email is absent', async () => {
+  test('Step 2: synthesizes noreply email when entry.email is absent', async () => {
     const reader = mockReader({});
     const store = makeTokenStore({ login: 'octocat' }); // no email
     const result = await resolveGitIdentity('/fake/repo', store, 'github.com', reader);
@@ -131,41 +95,41 @@ describe('resolveGitIdentity chain order', () => {
     });
   });
 
-  test('Step 5: returns null when all sources are empty', async () => {
+  test('Step 3: returns null when all sources are empty', async () => {
     const reader = mockReader({});
     const result = await resolveGitIdentity('/fake/repo', null, null, reader);
     expect(result).toBeNull();
   });
 
-  test('Step 5: returns null when tokenStore.get returns null', async () => {
+  test('Step 3: returns null when tokenStore.get returns null', async () => {
     const reader = mockReader({});
     const store = makeTokenStore(null);
     const result = await resolveGitIdentity('/fake/repo', store, 'github.com', reader);
     expect(result).toBeNull();
   });
 
-  test('Step 4 skipped: no host provided — falls through to null', async () => {
+  test('Step 2 skipped: no host provided — falls through to null', async () => {
     const reader = mockReader({});
     const store = makeTokenStore({
       login: 'octocat',
       name: 'The Octocat',
       email: 'cat@github.com',
     });
-    // host not provided — step 3 skipped even with a valid store
+    // host not provided — token step skipped even with a valid store
     const result = await resolveGitIdentity('/fake/repo', store, null, reader);
     expect(result).toBeNull();
   });
 
-  test('Step 4 skipped: no tokenStore — falls through to null', async () => {
+  test('Step 2 skipped: no tokenStore — falls through to null', async () => {
     const reader = mockReader({});
     const result = await resolveGitIdentity('/fake/repo', null, 'github.com', reader);
     expect(result).toBeNull();
   });
 
-  test('Local (Step 2) wins over global + tokenStore when set', async () => {
+  test('Git config (Step 1) wins over tokenStore when set', async () => {
     const reader = mockReader({
-      'user.name': { local: 'Repo Dev', global: 'Global Dev' },
-      'user.email': { local: 'repo@example.com', global: 'global@example.com' },
+      'user.name': 'Repo Dev',
+      'user.email': 'repo@example.com',
     });
     const store = makeTokenStore({
       login: 'octocat',