fix(open-knowledge): share-receive must not target ghost directories (#2682)

.changeset/share-receive-ghost-candidate.md

@@ -0,0 +1,5 @@
+---
+"@inkeep/open-knowledge-desktop": patch
+---
+
+Fix: opening a share link no longer offers to initialize a folder that is no longer a git checkout. If a project was moved or deleted outside OK, its recents entry could still be presented as the share target with a "this branch is checked out in <path>" claim and an "Initialize it and open?" prompt, because the path still existed on disk. Share receive now requires a candidate to be a real git working tree at that exact path, so a stale entry falls through to the launcher's "Clone from GitHub" / "I already have it locally" choices instead. Stale entries also no longer suppress worktree enumeration, so a share whose branch is checked out in a linked worktree now opens that worktree directly rather than prompting to switch branches.

packages/core/src/index.ts

@@ -1233,6 +1233,7 @@ export {
   findRecentProjectsForRepo,
   type HeadBranchInfo,
   InvalidShareUrlError,
+  isGitWorkingTree,
   type RecentProjectEntry,
   type ResolvedGitDirKind,
   selectCandidate,

packages/core/src/sharing/candidate-selection.test.ts

@@ -496,6 +496,67 @@ describe('selectCandidate', () => {
     }
   });
 
+  test('legacy soft-match: sole candidate that is a linked worktree still silent-dispatches', async () => {
+    // The working-tree guard on the soft-match must admit BOTH working-tree
+    // kinds. Narrowing it to `'directory'` would silently miss every share
+    // whose only recent is a linked worktree — the multi-worktree users this
+    // flow exists to serve. Every other soft-match case here is 'directory',
+    // and multi-candidate cases bypass the soft-match entirely, so without
+    // this case that narrowing passes the whole suite.
+    const wt = recent({ path: '/wt/feat' });
+    const bridge = makeBridge({
+      recents: [wt],
+      headsByPath: { '/wt/feat': { currentBranch: null, headSha: null, detached: false } },
+      gitDirKindByPath: { '/wt/feat': 'linked' },
+      okProjectRoots: new Set(['/wt/feat']),
+    });
+    const result = await selectCandidate(PAYLOAD, bridge);
+    expect(result.kind).toBe('branch-match-ok');
+    if (result.kind === 'branch-match-ok') {
+      expect(result.candidate.path).toBe('/wt/feat');
+    }
+  });
+
+  test('sole ghost candidate (gitDirKind absent, no .ok/config.yml) is never crowned a branch-match', async () => {
+    // A recents entry pointing at a directory that exists on disk but is NOT
+    // a git checkout and NOT an OK project — only OK's own droppings remain
+    // (the folder was moved in Finder and the still-running server recreated
+    // the path to write logs). Its HEAD is unreadable (no .git →
+    // currentBranch null, the all-null sentinel), gitDirKind is 'absent', and
+    // .ok/config.yml is absent. The share carries a branch.
+    //
+    // Selection must not present a non-checkout as the share target with a
+    // "this branch is checked out here" claim: a branch-match-* result must
+    // guarantee the candidate is a real git working tree. With the ghost as
+    // the sole candidate the correct outcome is a miss, not a soft-match.
+    const ghost = recent({ path: '/moved-away/CollaborationUX' });
+    const bridge = makeBridge({
+      recents: [ghost],
+      // No headsByPath entry → default all-null sentinel (unreadable HEAD).
+      // No gitDirKindByPath entry → default 'absent' (no .git on disk).
+      // Empty okProjectRoots → hasOkConfig false (no .ok/config.yml).
+    });
+    const result = await selectCandidate(PAYLOAD, bridge);
+    expect(result).toEqual({ kind: 'miss' });
+  });
+
+  test('sole candidate with a malformed-pointer or inaccessible .git is never crowned a branch-match', async () => {
+    // Same contract as the absent-ghost case across the rest of the
+    // non-working-tree kind set: a corrupt .git pointer (stale linked-worktree
+    // pointer) and an unreadable .git (EACCES) are equally not dispatchable
+    // checkouts, so the soft-match must refuse them too.
+    for (const kind of ['malformed-pointer', 'inaccessible'] as const) {
+      const ghost = recent({ path: `/degraded/${kind}` });
+      const bridge = makeBridge({
+        recents: [ghost],
+        gitDirKindByPath: { [`/degraded/${kind}`]: kind },
+        // No headsByPath entry → all-null sentinel (unreadable HEAD).
+      });
+      const result = await selectCandidate(PAYLOAD, bridge);
+      expect(result).toEqual({ kind: 'miss' });
+    }
+  });
+
   test('prunable worktree-enum entry on the share branch is NOT selected', async () => {
     // A prunable worktree (stale gitdir pointer, dir still on disk) reports a
     // matching `branch` via porcelain and would strict-match the share

packages/core/src/sharing/candidate-selection.ts

@@ -9,10 +9,11 @@
  * worktree only when no main checkout is available; returns miss when no
  * usable candidate exists.
  *
- * Pure module — no IPC, no React, no I/O, no node:*. Lives in core so both
- * the renderer (the share-receive dialog) and main (Electron's url-scheme
- * router) can call it with the same algorithm. Bridge surface comes through
- * `CandidateBridgeDeps` so tests stub the IPC reads via pure DI.
+ * Pure module — no IPC, no React, no I/O, no node:*. Lives in core alongside
+ * the rest of the share-receive decision primitives; main (Electron's
+ * url-scheme router) is the only production caller — the renderer consumes
+ * an already-resolved target rather than re-running selection. Bridge surface
+ * comes through `CandidateBridgeDeps` so tests stub the reads via pure DI.
  *
  * Selection rules:
  *  - Branch-match wins: the candidate whose `head.currentBranch ===
@@ -103,7 +104,7 @@ export interface Candidate {
   readonly recent: RecentProjectEntry | null;
   /** `.git/HEAD` read result. `{currentBranch:null, headSha:null, detached:false}` on graceful-fail. */
   readonly head: HeadBranchInfo;
-  /** `<path>/.git` classification. `'absent'` covers absent, malformed, inaccessible. */
+  /** `<path>/.git` classification, path-exact. See `isGitWorkingTree` for which kinds are dispatchable. */
   readonly gitDirKind: ResolvedGitDirKind;
   /** True iff `<path>/.ok/config.yml` exists as a regular file. */
   readonly hasOkConfig: boolean;
@@ -172,6 +173,21 @@ export type CandidateSelection =
     }
   | { readonly kind: 'miss' };
 
+/**
+ * True iff `kind` classifies a real git working tree — a main checkout
+ * (`'directory'`) or a linked worktree (`'linked'`). A candidate that is not
+ * a working tree can never legitimately be a branch-match: "a branch is
+ * checked out here" is false for a path with no `.git` (`'absent'`), a
+ * malformed pointer, or an inaccessible `.git`.
+ *
+ * Only as strong as its producer: the guarantee holds iff `kind` came from a
+ * probe that classifies `<path>/.git` itself rather than the nearest
+ * ancestor's. Main's `readGitDirKind` is that probe.
+ */
+export function isGitWorkingTree(kind: ResolvedGitDirKind): kind is 'directory' | 'linked' {
+  return kind === 'directory' || kind === 'linked';
+}
+
 /**
  * Pick the best candidate for `payload` from Recents + worktree enumeration.
  * See module docstring for the selection rules.
@@ -231,14 +247,23 @@ export async function selectCandidate(
   // and the all-null sentinel from a thrown `readHeadBranch`. With one
   // candidate the user has exactly one option so we cannot pick wrong;
   // with multiple candidates strict matching keeps the true match decisive.
+  // The soft-match still requires a real git working tree: `classifyBranchMatch`
+  // returns 'true' for a null HEAD (the "couldn't determine" sentinel — it
+  // fires on any `readHeadBranch` failure, including when there is no git repo
+  // at all), which would otherwise crown a directory with no `.git` at all —
+  // a branch-match must guarantee the candidate is an actual checkout.
   const strictMatches = candidates.filter(
     (c) => c.head.currentBranch !== null && c.head.currentBranch === payload.branch,
   );
   const branchMatches =
     strictMatches.length > 0
       ? strictMatches
       : candidates.length === 1
-        ? candidates.filter((c) => classifyBranchMatch(payload.branch, c.head) === 'true')
+        ? candidates.filter(
+            (c) =>
+              isGitWorkingTree(c.gitDirKind) &&
+              classifyBranchMatch(payload.branch, c.head) === 'true',
+          )
         : [];
   if (branchMatches.length > 0) {
     const chosen = pickByRecency(branchMatches);
@@ -265,12 +290,24 @@ export async function selectCandidate(
   // Only OK-initialized candidates can take the branch-switch dispatch path.
   const mains = candidates.filter((c) => c.gitDirKind === 'directory' && c.hasOkConfig);
   if (mains.length > 0) {
+    console.warn(
+      `[receive] selection=fallback reason=main-checkout candidates=${candidates.length}`,
+    );
     return { kind: 'fallback', anchor: pickByRecency(mains), reason: 'main-checkout' };
   }
   const linkedOk = candidates.filter((c) => c.gitDirKind === 'linked' && c.hasOkConfig);
   if (linkedOk.length > 0) {
+    console.warn(
+      `[receive] selection=fallback reason=only-worktrees candidates=${candidates.length}`,
+    );
     return { kind: 'fallback', anchor: pickByRecency(linkedOk), reason: 'only-worktrees' };
   }
+  // The counts (never paths — PII discipline) make a refused-candidate miss
+  // distinguishable from a no-recents miss in triage: candidates>0 here means
+  // every candidate was refused (non-checkout, or checkout without OK config).
+  console.warn(
+    `[receive] selection=miss reason=no-usable-candidate candidates=${candidates.length}`,
+  );
   return { kind: 'miss' };
 }
 

packages/core/src/sharing/index.ts

@@ -3,6 +3,7 @@ export {
   type CandidateBridgeDeps,
   type CandidateSelection,
   type CandidateSelectionPayload,
+  isGitWorkingTree,
   selectCandidate,
 } from './candidate-selection.ts';
 export {

packages/core/src/sharing/receive-flow.ts

@@ -1,10 +1,10 @@
 /**
  * Pure share-receive helpers + the shared types they read.
  *
- * Lives in core so both the renderer (the share-receive dialog) and main
- * (`url-scheme.ts`'s `routeShare`) can run the same selection algorithm
- * against the same data shapes — `packages/desktop` cannot import from
- * `packages/app`, so any code that has to run on both sides ends up here.
+ * Lives in core so the selection algorithm and the data shapes it reads stay
+ * in one place, independent of which process runs them. Main (`url-scheme.ts`'s
+ * `routeShare`) is the only production caller today — the renderer consumes an
+ * already-resolved share target rather than re-running selection.
  *
  * No IPC, no React, no I/O, no `node:*` — browser+Node pure. Tests stub the
  * bridge surface directly.
@@ -22,11 +22,20 @@ export interface HeadBranchInfo {
 }
 
 /**
- * Discriminator returned by `bridge.project.readGitDirKind(path)`:
- * `'directory'` is a real `.git/` (main checkout — safe to `git checkout`),
- * `'linked'` is a `.git` file pointing at a worktree's gitdir, `'absent'` is
- * no `.git`, `'malformed-pointer'` is a `.git` file that doesn't parse, and
- * `'inaccessible'` is a `.git` that exists but can't be read.
+ * Classification of `<path>/.git`. A producer MUST answer the path-exact
+ * question — "is a branch checked out at THIS path" — classifying `<path>/.git`
+ * itself and never an ancestor's, and never reporting a working tree it has not
+ * verified is one. Selection's guarantee that a branch-match is a real checkout
+ * holds only as far as that obligation does. Main's `readGitDirKind` is the
+ * reference implementation.
+ *
+ * `'directory'` is a real `.git/` (main checkout — safe to `git checkout`) and
+ * `'linked'` is a `.git` file pointing at a live worktree gitdir; these two are
+ * the working trees. `'absent'` means no working tree at this exact path: no
+ * `.git`, or a `.git` belonging only to an ancestor, or a `.git/` with no HEAD.
+ * `'malformed-pointer'` is a `.git` file that doesn't parse, or one that parses
+ * but whose target gitdir is gone. `'inaccessible'` is a `.git` that exists but
+ * can't be read.
  */
 export type ResolvedGitDirKind =
   | 'directory'

packages/desktop/src/main/read-git-dir-kind.test.ts

@@ -1,6 +1,14 @@
 import { afterEach, describe, expect, test } from 'bun:test';
 import { execFile } from 'node:child_process';
-import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
+import {
+  chmodSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  realpathSync,
+  rmSync,
+  writeFileSync,
+} from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { promisify } from 'node:util';
@@ -51,4 +59,58 @@ describe('readGitDirKind', () => {
     await git(mainRepo, 'worktree', 'add', '-b', 'feat', wt);
     expect(readGitDirKind(wt)).toBe('linked');
   });
+
+  test('returns "absent" for a shell .git/ that holds only a shadow repo, not a checkout', () => {
+    // Fixture is what `initShadowRepo` itself leaves behind on a no-repo path.
+    testRoot = realpathSync(mkdtempSync(join(tmpdir(), 'gitdir-kind-')));
+    mkdirSync(join(testRoot, '.git', 'ok'), { recursive: true });
+    expect(readGitDirKind(testRoot)).toBe('absent');
+  });
+
+  test('returns "malformed-pointer" for a linked worktree whose admin gitdir is gone', () => {
+    // Fixture is a pointer that parses cleanly but targets nothing — what an
+    // `rm -rf` of the admin dir without `git worktree prune` leaves.
+    testRoot = realpathSync(mkdtempSync(join(tmpdir(), 'gitdir-kind-')));
+    writeFileSync(join(testRoot, '.git'), `gitdir: ${join(testRoot, 'gone', 'worktrees', 'x')}\n`);
+    expect(readGitDirKind(testRoot)).toBe('malformed-pointer');
+  });
+
+  test('returns "absent" for a plain subfolder of a real repo (the .git is the ancestor\'s)', async () => {
+    // Fixture has no `.git` of its own, so the walk-up finds the repo's.
+    testRoot = realpathSync(mkdtempSync(join(tmpdir(), 'gitdir-kind-')));
+    await git(testRoot, 'init', '--initial-branch=main', '.');
+    const sub = join(testRoot, 'subfolder');
+    mkdirSync(sub);
+    expect(readGitDirKind(sub)).toBe('absent');
+  });
+
+  test('returns "inaccessible" for a real checkout whose gitdir cannot be traversed', () => {
+    // A HEAD we cannot look at is not the same as a HEAD that isn't there:
+    // this must not collapse onto the moved-away-ghost classification, or the
+    // share path's drop log can no longer tell an ACL problem from a folder
+    // the user moved.
+    testRoot = realpathSync(mkdtempSync(join(tmpdir(), 'gitdir-kind-')));
+    const gitDir = join(testRoot, '.git');
+    mkdirSync(gitDir, { recursive: true });
+    writeFileSync(join(gitDir, 'HEAD'), 'ref: refs/heads/main\n');
+    chmodSync(gitDir, 0o000);
+    // Skip where chmod 0o000 is bypassed (root); the assertion would be vacuous.
+    let stillReadable = false;
+    try {
+      readFileSync(join(gitDir, 'HEAD'), 'utf-8');
+      stillReadable = true;
+    } catch {
+      // expected — the mode bits should refuse traversal
+    }
+    if (stillReadable) {
+      chmodSync(gitDir, 0o755);
+      return;
+    }
+    try {
+      expect(readGitDirKind(testRoot)).toBe('inaccessible');
+    } finally {
+      // Restore before afterEach's rmSync, which cannot recurse into 0o000.
+      chmodSync(gitDir, 0o755);
+    }
+  });
 });