diff --git a/.changeset/share-receive-ghost-candidate.md b/.changeset/share-receive-ghost-candidate.md new file mode 100644 index 000000000..15d6540ee --- /dev/null +++ b/.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 " 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. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 14c0d0688..3032b2a76 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1233,6 +1233,7 @@ export { findRecentProjectsForRepo, type HeadBranchInfo, InvalidShareUrlError, + isGitWorkingTree, type RecentProjectEntry, type ResolvedGitDirKind, selectCandidate, diff --git a/packages/core/src/sharing/candidate-selection.test.ts b/packages/core/src/sharing/candidate-selection.test.ts index 247613c9f..daa322ed1 100644 --- a/packages/core/src/sharing/candidate-selection.test.ts +++ b/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 diff --git a/packages/core/src/sharing/candidate-selection.ts b/packages/core/src/sharing/candidate-selection.ts index 23da97676..cf468e1a1 100644 --- a/packages/core/src/sharing/candidate-selection.ts +++ b/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; - /** `/.git` classification. `'absent'` covers absent, malformed, inaccessible. */ + /** `/.git` classification, path-exact. See `isGitWorkingTree` for which kinds are dispatchable. */ readonly gitDirKind: ResolvedGitDirKind; /** True iff `/.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 `/.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,6 +247,11 @@ 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, ); @@ -238,7 +259,11 @@ export async function selectCandidate( 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' }; } diff --git a/packages/core/src/sharing/index.ts b/packages/core/src/sharing/index.ts index 25da97995..0a50bc5f4 100644 --- a/packages/core/src/sharing/index.ts +++ b/packages/core/src/sharing/index.ts @@ -3,6 +3,7 @@ export { type CandidateBridgeDeps, type CandidateSelection, type CandidateSelectionPayload, + isGitWorkingTree, selectCandidate, } from './candidate-selection.ts'; export { diff --git a/packages/core/src/sharing/receive-flow.ts b/packages/core/src/sharing/receive-flow.ts index 2230613c9..8e8b7310e 100644 --- a/packages/core/src/sharing/receive-flow.ts +++ b/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 `/.git`. A producer MUST answer the path-exact + * question — "is a branch checked out at THIS path" — classifying `/.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' diff --git a/packages/desktop/src/main/read-git-dir-kind.test.ts b/packages/desktop/src/main/read-git-dir-kind.test.ts index 3523ff13e..eb898556a 100644 --- a/packages/desktop/src/main/read-git-dir-kind.test.ts +++ b/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); + } + }); }); diff --git a/packages/desktop/src/main/read-git-dir-kind.ts b/packages/desktop/src/main/read-git-dir-kind.ts index 51c685fa4..845f55cae 100644 --- a/packages/desktop/src/main/read-git-dir-kind.ts +++ b/packages/desktop/src/main/read-git-dir-kind.ts @@ -1,25 +1,40 @@ /** - * Renderer-callable wrapper around `resolveGitDirDetailed` from core. Returns - * just the discriminator string (`'directory'` / `'linked'` / `'absent'` / - * `'malformed-pointer'` / `'inaccessible'`) so the IPC payload stays small - * and JSON-stable across the wire. + * Classify `/.git` — is a branch checked out at THIS exact path? + * Returns just the discriminator string (`'directory'` / `'linked'` / + * `'absent'` / `'malformed-pointer'` / `'inaccessible'`). * - * Used by the share-receive Q1 candidate-selection step to partition - * Candidates into "main checkouts" (`'directory'`) vs "linked worktrees" - * (`'linked'`) for the no-branch-match fallback — selection prefers main - * checkouts over worktrees because switching main is safe; switching a - * worktree off its branch defeats the worktree's purpose. + * Used by the share-receive candidate-selection step, both to admit share + * candidates at all and to partition them into "main checkouts" + * (`'directory'`) vs "linked worktrees" (`'linked'`) for the no-branch-match + * fallback — selection prefers main checkouts over worktrees because + * switching main is safe; switching a worktree off its branch defeats the + * worktree's purpose. * - * Never throws — every input-rejection or filesystem error collapses to - * `'absent'` so the caller treats the candidate as a no-`.git`-here - * fall-through. The pure `resolveGitDirDetailed` already returns - * `'malformed-pointer'` and `'inaccessible'` as discriminated values, so the - * caller can distinguish them when useful; selection treats all three - * non-`'directory'`/`'linked'` values identically (skip the candidate in - * fallback partitioning). + * `resolveGitDirDetailed` answers a DIFFERENT question — "which gitdir hosts + * this path's shadow repo?" — whose correct answer for a subfolder is the + * ancestor's gitdir, and which never verifies the `.git` it found is a real + * working tree. Three real on-disk states diverge, so each is narrowed here: + * + * - `.git` found at an ANCESTOR (non-empty `projectSubPath`): no branch is + * checked out at this path. Same `projectSubPath === ''` idiom as + * `folder-admission.ts`'s linked-worktree carveout. + * - `.git` pointer whose admin gitdir is gone: a stale worktree pointer. + * `resolveShadowDir` already classifies this state as + * `MalformedGitPointerError`. + * - `.git/` directory with no `HEAD`: a shell `.git/` holding only a shadow + * repo. `resolveShadowDir`'s no-`.git` fallthrough returns + * `/.git/ok` and `initShadowRepo` mkdir -p's it, so OK itself + * creates this shape whenever a server boots against a path with no repo. + * + * Never throws: an input rejection or an unresolvable `.git` yields `'absent'`; + * a `.git` that exists but can't be read yields `'inaccessible'`. Callers can + * distinguish `'malformed-pointer'` and `'inaccessible'` when useful; selection + * treats all three non-`'directory'`/`'linked'` values identically (refuse the + * candidate). */ -import { isAbsolute } from 'node:path'; +import { statSync } from 'node:fs'; +import { isAbsolute, join } from 'node:path'; import { resolveGitDirDetailed } from '@inkeep/open-knowledge-core/shadow-repo-layout'; /** @@ -34,10 +49,40 @@ export type ResolvedGitDirKind = | 'malformed-pointer' | 'inaccessible'; +/** + * Does the resolved gitdir hold a `HEAD`? Discriminates on errno rather than + * `existsSync`, which reports `false` for an unreadable `.git` exactly as it + * does for a shell one — collapsing a real checkout behind restrictive ACLs + * (SMB/NFS mount, another user's clone, a broken mode) onto the + * moved-away-ghost classification. Same ENOENT/ENOTDIR split `classifyGitEntry` + * applies one level up. + */ +function readHeadState(gitDir: string): 'present' | 'missing' | 'inaccessible' { + try { + statSync(join(gitDir, 'HEAD')); + return 'present'; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return code === 'ENOENT' || code === 'ENOTDIR' ? 'missing' : 'inaccessible'; + } +} + export function readGitDirKind(projectPath: string): ResolvedGitDirKind { if (!isAbsolute(projectPath)) return 'absent'; try { - return resolveGitDirDetailed(projectPath).kind; + const resolved = resolveGitDirDetailed(projectPath); + if (resolved.kind === 'directory' || resolved.kind === 'linked') { + if (resolved.projectSubPath !== '') return 'absent'; + try { + statSync(resolved.path); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return code === 'ENOENT' || code === 'ENOTDIR' ? 'malformed-pointer' : 'inaccessible'; + } + const head = readHeadState(resolved.path); + if (head !== 'present') return head === 'missing' ? 'absent' : 'inaccessible'; + } + return resolved.kind; } catch { return 'absent'; } diff --git a/packages/desktop/src/main/resolve-share-target.test.ts b/packages/desktop/src/main/resolve-share-target.test.ts index 5b3ad8d99..2966d1e55 100644 --- a/packages/desktop/src/main/resolve-share-target.test.ts +++ b/packages/desktop/src/main/resolve-share-target.test.ts @@ -5,7 +5,8 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { promisify } from 'node:util'; import type { RecentProjectEntry } from '@inkeep/open-knowledge-core'; -import { resolveShareTarget } from './resolve-share-target.ts'; +import { filterShareEligibleRecents, resolveShareTarget } from './resolve-share-target.ts'; +import { annotateMissing, emptyState } from './state-store.ts'; const execFileAsync = promisify(execFile); @@ -164,6 +165,129 @@ describe('resolveShareTarget — main-side adapter parity with the shared algori expect(selection).toEqual({ kind: 'miss' }); }); + test('a ghost most-recent entry does not poison the worktree-enumeration anchor', async () => { + // The behavior only the recents filter provides: it runs upstream of + // findRecentProjectsForRepo, so it is what keeps the ghost from becoming + // recentMatches[0] and rooting listGitWorktrees at a non-repo cwd. Drop + // the filter and this share lands on `fallback`/main-checkout — a + // branch-switch dialog — even though the branch is already checked out in + // a worktree. The downstream soft-match guard runs after the anchor is + // chosen and cannot rescue this. + handle = await makeRepoWithWorktrees(['feat-foo']); + const wtPath = handle.worktrees.get('feat-foo'); + expect(wtPath).toBeDefined(); + if (!wtPath) return; + seedOkProject(handle.mainRepo); + seedOkProject(wtPath); + + const ghostPath = join(handle.root, 'CollaborationUX'); + mkdirSync(join(ghostPath, '.ok', 'local', 'logs'), { recursive: true }); + writeFileSync(join(ghostPath, '.ok', 'local', 'logs', 'server-current.jsonl'), '{}\n'); + + const selection = await resolveShareTarget(PAYLOAD, { + // Ghost is the most recently opened, so it would be the anchor. + listRecent: () => [recent(ghostPath), recent(handle?.mainRepo ?? '')], + }); + + expect(selection.kind).toBe('branch-match-ok'); + if (selection.kind === 'branch-match-ok') { + expect(selection.candidate.path).toBe(wtPath); + } + }); + + test('filterShareEligibleRecents drops a ghost directory (no .git) but keeps a real git working tree', async () => { + // The share-admission predicate must refuse a moved-away directory that + // survives the bare-existence missing filter — it exists on disk but holds + // only OK's own droppings (no .git, no .ok/config.yml) — while still + // admitting a real checkout. Uses the real on-disk readGitDirKind against + // real fixtures. + handle = await makeRepoWithWorktrees([]); + const ghostRoot = realpathSync(mkdtempSync(join(tmpdir(), 'eligible-ghost-'))); + try { + const ghostPath = join(ghostRoot, 'CollaborationUX'); + mkdirSync(join(ghostPath, '.ok', 'local', 'logs'), { recursive: true }); + writeFileSync(join(ghostPath, '.ok', 'local', 'logs', 'server-current.jsonl'), '{}\n'); + + const ghostEntry = recent(ghostPath); + const liveEntry = recent(handle.mainRepo); + + const eligible = filterShareEligibleRecents([ghostEntry, liveEntry]); + + expect(eligible.map((e) => e.path)).toEqual([handle.mainRepo]); + } finally { + rmSync(ghostRoot, { recursive: true, force: true }); + } + }); + + test('filterShareEligibleRecents drops a stale worktree pointer (.git file to a gone gitdir)', async () => { + // The moved-away-linked-worktree ghost shape: the directory still holds a + // `.git` FILE, but its pointer targets an admin gitdir that no longer + // exists. Not a dispatchable checkout — must be refused like the + // no-.git ghost. + handle = await makeRepoWithWorktrees([]); + const staleRoot = realpathSync(mkdtempSync(join(tmpdir(), 'eligible-stale-'))); + try { + const stalePath = join(staleRoot, 'CollaborationUX'); + mkdirSync(stalePath, { recursive: true }); + writeFileSync( + join(stalePath, '.git'), + `gitdir: ${join(staleRoot, 'gone', '.git', 'worktrees', 'x')}\n`, + ); + + const staleEntry = recent(stalePath); + const liveEntry = recent(handle.mainRepo); + + const eligible = filterShareEligibleRecents([staleEntry, liveEntry]); + + expect(eligible.map((e) => e.path)).toEqual([handle.mainRepo]); + } finally { + rmSync(staleRoot, { recursive: true, force: true }); + } + }); + + test('ghost recents entry (exists, no .git, no .ok/config.yml) resolves to miss through the production annotateMissing wiring', async () => { + // Reproduces the full share-receive ghost scenario end to end: a recents + // entry still carries the repo's gitRemoteUrl from when the project was + // real at this path, but the path now holds only OK's own droppings (the + // folder was moved in Finder and the still-running server recreated it to + // write logs). The share carries a branch. A ghost directory that is + // neither a git checkout nor an OK project must never be presented as the + // share target with a "this branch is checked out here" claim — selection + // must return a miss. + const root = realpathSync(mkdtempSync(join(tmpdir(), 'resolve-share-ghost-'))); + handle = { + root, + mainRepo: root, + worktrees: new Map(), + cleanup: () => rmSync(root, { recursive: true, force: true }), + }; + const ghostPath = join(root, 'CollaborationUX'); + mkdirSync(join(ghostPath, '.ok', 'local', 'logs'), { recursive: true }); + writeFileSync(join(ghostPath, '.ok', 'local', 'logs', 'server-current.jsonl'), '{}\n'); + + const state = { + ...emptyState(), + recentProjects: [ + { + path: ghostPath, + name: 'CollaborationUX', + lastOpenedAt: '2026-07-15T00:00:00.000Z', + gitRemoteUrl: 'https://github.com/acme/widget.git', + }, + ], + }; + + const selection = await resolveShareTarget(PAYLOAD, { + // Production wiring: the share path feeds selection the annotateMissing + // projection, which marks this ghost non-missing because the directory + // does exist — so it reaches the share-eligibility filter rather than + // being dropped upstream of it. + listRecent: () => annotateMissing(state), + }); + + expect(selection).toEqual({ kind: 'miss' }); + }); + test('parity: real git I/O + real isProjectRoot reproduces the renderer outcome shape for an OK worktree on the shared branch', async () => { handle = await makeRepoWithWorktrees(['feat-foo', 'feat-bar']); const featFoo = handle.worktrees.get('feat-foo'); diff --git a/packages/desktop/src/main/resolve-share-target.ts b/packages/desktop/src/main/resolve-share-target.ts index e8adcb203..c3e48c1d3 100644 --- a/packages/desktop/src/main/resolve-share-target.ts +++ b/packages/desktop/src/main/resolve-share-target.ts @@ -3,12 +3,11 @@ * main-process git primitives (plus a caller-supplied recents lister) and runs * `selectCandidate` natively — no IPC round-trip, no renderer required. * - * Same algorithm the renderer's `ShareReceiveDialog` runs (the algorithm - * itself lives in `@inkeep/open-knowledge-core` so both sides share one - * implementation). This module is the bridge wiring main needs so the share - * target can be resolved BEFORE any window opens — eliminating the - * "launcher flash" and replacing the focused-window dispatch in `routeShare` - * with a routed-by-outcome decision. + * The algorithm itself lives in `@inkeep/open-knowledge-core`; this module is + * the bridge wiring main needs so the share target can be resolved BEFORE any + * window opens — eliminating the "launcher flash" and replacing the + * focused-window dispatch in `routeShare` with a routed-by-outcome decision. + * The renderer consumes the resolved outcome and does not re-run selection. * * Two main-process primitives (`readHeadBranch`, `readGitDirKind`) are * synchronous; the bridge contract returns Promises. They're wrapped in @@ -35,12 +34,13 @@ import { type CandidateBridgeDeps, type CandidateSelection, type CandidateSelectionPayload, + isGitWorkingTree, type RecentProjectEntry, selectCandidate, } from '@inkeep/open-knowledge-core'; import { isProjectRoot } from '@inkeep/open-knowledge-server'; import { listGitWorktrees } from './list-git-worktrees.ts'; -import { readGitDirKind } from './read-git-dir-kind.ts'; +import { type ResolvedGitDirKind, readGitDirKind } from './read-git-dir-kind.ts'; import { readHeadBranch } from './read-head-branch.ts'; export interface MainShareTargetDeps { @@ -54,6 +54,56 @@ export interface MainShareTargetDeps { readonly listRecent: () => readonly RecentProjectEntry[]; } +/** + * Keep only recents entries whose path still resolves to a real git working + * tree — a moved-away directory that survives the bare-existence `missing` + * filter (only OK's own droppings remain: no `.git`, no `.ok/config.yml`) is + * refused here. + * + * Runs upstream of `findRecentProjectsForRepo` — that placement is the point. + * It is what keeps a ghost from becoming `recentMatches[0]`, the anchor that + * roots `listGitWorktrees`. A ghost anchor makes `git` fail from a non-repo + * cwd, so the real repo's worktrees are never enumerated and a share whose + * branch is checked out in a worktree lands on a branch-switch dialog instead. + * Nothing downstream can undo that — by then the anchor is chosen. + * + * Its relationship to the working-tree guard on `selectCandidate`'s + * single-candidate soft-match is asymmetric, not mutual. That guard cannot + * reach the anchor. This filter, however, does reach the crown on the + * production path: `buildCandidateSet` seeds from `recentMatches`, so a sole + * candidate is always a Recents entry that already passed here. The core guard + * earns its place for reasons independent of this filter — core owes the + * "a branch-match is a real checkout" invariant to every `CandidateBridgeDeps` + * implementor, not just this bridge, and the two probes differ in path and + * time: this one reads `entry.path` raw, `inspectCandidate` re-probes the + * realpath later, catching a folder moved in between. + * + * Drops are logged by `.git` classification (never the path — PII discipline, + * matching the `safe*` wrappers in `selectCandidate`). A drop can flip the + * user-visible outcome to a launcher miss, and `'inaccessible'` (EACCES on a + * real `.git` — sandbox, SMB mount) degrades identically to a genuinely + * moved-away folder, so the classification is what makes the two + * distinguishable in triage. + */ +export function filterShareEligibleRecents( + recents: readonly RecentProjectEntry[], +): RecentProjectEntry[] { + const dropped: ResolvedGitDirKind[] = []; + const eligible = recents.filter((entry) => { + const kind = readGitDirKind(entry.path); + if (isGitWorkingTree(kind)) return true; + dropped.push(kind); + return false; + }); + if (dropped.length > 0) { + console.warn('[receive] recents_filtered reason=not_git_working_tree', { + dropped: dropped.length, + kinds: [...new Set(dropped)].sort(), + }); + } + return eligible; +} + /** * Build the `CandidateBridgeDeps` the shared `selectCandidate` algorithm * expects, backed by real main-side git I/O. The returned object is stateless @@ -62,7 +112,7 @@ export interface MainShareTargetDeps { */ function createMainCandidateBridge(deps: MainShareTargetDeps): CandidateBridgeDeps { return { - listRecent: async () => deps.listRecent(), + listRecent: async () => filterShareEligibleRecents(deps.listRecent()), listGitWorktrees: (anchorPath) => listGitWorktrees(anchorPath), readHeadBranch: async (projectPath) => readHeadBranch(projectPath), readGitDirKind: async (projectPath) => readGitDirKind(projectPath), diff --git a/packages/desktop/src/shared/bridge-contract.ts b/packages/desktop/src/shared/bridge-contract.ts index a8bd81bf6..8701abd64 100644 --- a/packages/desktop/src/shared/bridge-contract.ts +++ b/packages/desktop/src/shared/bridge-contract.ts @@ -285,20 +285,6 @@ export interface HeadBranchInfo { readonly detached: boolean; } -/** - * Discriminator string returned by `bridge.project.readGitDirKind(path)`. - * Mirrors `ResolvedGitDir.kind` from `@inkeep/open-knowledge-core/shadow- - * repo-layout`. The candidate-selection algorithm cares about `'directory'` - * (main checkout) and `'linked'` (worktree); the other three values - * collapse to "skip in fallback partitioning." - */ -export type ResolvedGitDirKind = - | 'directory' - | 'linked' - | 'absent' - | 'malformed-pointer' - | 'inaccessible'; - /** * Outcome of `bridge.project.checkTargetExists({projectPath, kind, path})`. * See the bridge method's JSDoc for the failure-mode contract.