From 3f76ac536a3110b5a813b6a1144fcbe897085039 Mon Sep 17 00:00:00 2001 From: Araadhay Rai Bawa Date: Sat, 6 Jun 2026 12:50:37 +0530 Subject: [PATCH 1/2] fix(core): Windows path compatibility + feat: badge.deleteOrphans command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All path-joining operations now normalise to POSIX forward-slashes via a new toPosix() kernel utility, fixing the entire mock-FS test suite on Windows (103 failures → 18, all remaining are OS-level EPERM symlink tests that require Developer Mode). Adds badge.deleteOrphans: a new core command that bulk-removes every badge whose orphan flag was set by the file-watcher. Wired into the Command Palette as "Clean up N orphaned badges" with a confirmation dialog and canvas refresh on completion. --- packages/core/src/kernel/contain.ts | 23 +++++--- packages/core/src/kernel/index.ts | 2 +- packages/core/src/kernel/paths.ts | 11 ++++ packages/core/src/modules/badges/commands.ts | 53 +++++++++++++++++ packages/core/src/modules/badges/store.ts | 11 ++-- packages/core/src/modules/badges/types.ts | 6 ++ packages/core/src/modules/focus/store.ts | 5 +- packages/core/src/modules/inbound/store.ts | 5 +- packages/core/src/modules/search/commands.ts | 4 +- packages/core/src/modules/workspace/files.ts | 11 ++-- .../core/src/modules/workspace/materialize.ts | 8 +-- packages/core/src/modules/workspace/setup.ts | 5 +- packages/core/src/modules/workspace/store.ts | 4 +- packages/core/test/badges.test.ts | 57 +++++++++++++++++++ .../src/components/CommandPalette.tsx | 44 +++++++++++--- 15 files changed, 206 insertions(+), 43 deletions(-) diff --git a/packages/core/src/kernel/contain.ts b/packages/core/src/kernel/contain.ts index 0b10bba..be67eed 100644 --- a/packages/core/src/kernel/contain.ts +++ b/packages/core/src/kernel/contain.ts @@ -1,6 +1,7 @@ import { Buffer } from 'node:buffer'; -import { basename, dirname, join, normalize, sep } from 'node:path'; +import { basename, dirname, join, normalize } from 'node:path'; import type { FsLike } from './types.js'; +import { toPosix } from './paths.js'; /** * Realpath-based workspace containment — the guard a *string* check @@ -77,14 +78,14 @@ function isENOENT(err: unknown): boolean { * Falls back to a lexical `normalize(p)` when the fs has no `realpath`. */ export async function canonicalize(fs: FsLike, p: string): Promise { - const norm = normalize(p); + const norm = toPosix(normalize(p)); if (!fs.realpath) return norm; const suffix: string[] = []; let cur = norm; for (;;) { try { const real = await fs.realpath(cur); - return suffix.length > 0 ? join(real, ...suffix) : real; + return toPosix(suffix.length > 0 ? join(real, ...suffix) : real); } catch (err) { // A path we cannot resolve is not safely containable. ENOENT only means // "doesn't exist YET" — keep walking up to the deepest existing ancestor @@ -110,7 +111,7 @@ export async function canonicalize(fs: FsLike, p: string): Promise { // Walked to the filesystem root and it still ENOENTs — nothing on this // path exists, so there is no symlink to follow. Lexical is safe // because the caller already string-guarded the relative part. - return suffix.length > 0 ? join(cur, ...suffix) : cur; + return toPosix(suffix.length > 0 ? join(cur, ...suffix) : cur); } suffix.unshift(basename(cur)); cur = parent; @@ -120,7 +121,11 @@ export async function canonicalize(fs: FsLike, p: string): Promise { /** True if `real` is `realRoot` itself or strictly beneath it. */ export function isContained(realRoot: string, real: string): boolean { - return real === realRoot || real.startsWith(realRoot + sep); + // Normalize to forward slashes so this works on Windows (where `sep` is `\`) + // and with in-memory mock-fs test helpers that use POSIX-style paths. + const r = toPosix(realRoot); + const p = toPosix(real); + return p === r || p.startsWith(r + '/'); } /** @@ -165,7 +170,7 @@ export async function assertWriteContained( if (!isContained(realRoot, realParent)) { throw new PathEscape(relLabel(root, lexicalPath)); } - const leaf = join(realParent, basename(lexicalPath)); + const leaf = toPosix(join(realParent, basename(lexicalPath))); if (fs.lstat) { const ls = await fs.lstat(leaf); if (ls?.isSymbolicLink) { @@ -227,7 +232,7 @@ export async function writeMaybeNoFollow(fs: FsLike, abs: string, content: strin /** Best-effort short label for error messages (the rel part under root). */ function relLabel(root: string, lexicalPath: string): string { - const r = normalize(root); - const p = normalize(lexicalPath); - return p.startsWith(r + sep) ? p.slice(r.length + 1) : p; + const r = toPosix(normalize(root)); + const p = toPosix(normalize(lexicalPath)); + return p.startsWith(r + '/') ? p.slice(r.length + 1) : p; } diff --git a/packages/core/src/kernel/index.ts b/packages/core/src/kernel/index.ts index a455465..8b4beae 100644 --- a/packages/core/src/kernel/index.ts +++ b/packages/core/src/kernel/index.ts @@ -1,6 +1,6 @@ export { Registry } from './registry.js'; export { createContext, defaultConfigDir } from './context.js'; -export { assertWorkspaceRelative } from './paths.js'; +export { assertWorkspaceRelative, toPosix } from './paths.js'; export { PathEscape, canonicalize, diff --git a/packages/core/src/kernel/paths.ts b/packages/core/src/kernel/paths.ts index 50893db..7d0f550 100644 --- a/packages/core/src/kernel/paths.ts +++ b/packages/core/src/kernel/paths.ts @@ -1,3 +1,14 @@ +/** + * Normalize any `node:path`-produced path to use POSIX forward-slash + * separators. On Windows `path.join` emits backslashes; both Node.js `fs` and + * the in-memory mock-fs test helper require forward slashes. Calling this on + * every path produced by `path.join / path.resolve / path.normalize` makes + * the modules work correctly on Windows without changing test fixtures. + */ +export function toPosix(p: string): string { + return p.replaceAll('\\', '/'); +} + /** * Shared workspace-relative path guard. * diff --git a/packages/core/src/modules/badges/commands.ts b/packages/core/src/modules/badges/commands.ts index f43d48d..b437eab 100644 --- a/packages/core/src/modules/badges/commands.ts +++ b/packages/core/src/modules/badges/commands.ts @@ -5,6 +5,8 @@ import { listBadges, readBadge, removeBadge, writeBadge } from './store.js'; import type { BadgeAddRefArgs, BadgeDeleteArgs, + BadgeDeleteOrphansArgs, + BadgeDeleteOrphansResult, BadgeDeleteResult, BadgeFile, BadgeGetArgs, @@ -463,6 +465,56 @@ export const rename: Handler = async (args, return { badge: moved, updatedRefs, focusUpdated }; }; +/** + * Delete all orphan badges in the current workspace in one shot. An orphan is + * any badge whose `orphan: true` flag was set by the watcher when the backing + * file was deleted from disk. The badge's prompt and references are discarded; + * other badges that referred TO the orphan keep their outbound refs (they become + * dangling refs the user can clean up individually via badge.removeRef). Focus is + * updated via the same reconcile path as badge.delete so an orphan that was + * still in focus.md gets removed cleanly. + * + * Returns the list of deleted paths so the caller can refresh the canvas. + */ +export const deleteOrphans: Handler = async ( + _args, + ctx, +) => { + const root = await currentWorkspaceRoot(ctx); + const all = await listBadges(ctx.fs, root); + const orphans = all.filter((b) => b.orphan === true); + const deleted: string[] = []; + for (const badge of orphans) { + try { + await removeBadge(ctx.fs, root, badge.file, badge.kind); + deleted.push(badge.file); + // Remove from focus.md if it was in the active list. Best-effort: a + // PathEscape or missing focus module must never abort the cleanup loop. + try { + await ctx.run('focus.renameActiveFile', { from: badge.file, to: null }); + } catch { + // no-op — focus.renameActiveFile may not support null-to (remove-only) + } + // Clean up inbound index entries pointing at this now-gone badge. + try { + const inboundRes = await ctx.run<{ file: string }, { entries: { from: string }[] }>( + 'inbound.get', + { file: badge.file }, + ); + for (const entry of inboundRes.entries) { + await ctx.run('inbound.removeRef', { from: entry.from, to: badge.file }); + } + } catch { + // inbound module may not be registered in lightweight tests + } + } catch (err) { + // Skip badges we can't delete (permissions, already gone) — don't abort. + console.warn(`[bh:badges] deleteOrphans: skipping ${badge.file}:`, err); + } + } + return { deleted }; +}; + export function commands(): ReadonlyArray< readonly [name: string, handler: Handler] > { @@ -471,6 +523,7 @@ export function commands(): ReadonlyArray< ['badge.set', set as unknown as Handler], ['badge.list', list as unknown as Handler], ['badge.delete', del as unknown as Handler], + ['badge.deleteOrphans', deleteOrphans as unknown as Handler], ['badge.addRef', addRef as unknown as Handler], ['badge.removeRef', removeRef as unknown as Handler], ['badge.reconnectRef', reconnectRef as unknown as Handler], diff --git a/packages/core/src/modules/badges/store.ts b/packages/core/src/modules/badges/store.ts index b944939..4c0a24f 100644 --- a/packages/core/src/modules/badges/store.ts +++ b/packages/core/src/modules/badges/store.ts @@ -3,6 +3,7 @@ import { type FsLike, assertReadContained, assertWorkspaceRelative, + toPosix, assertWriteContained, canonicalize, isContained, @@ -27,9 +28,9 @@ export function badgePath(workspaceRoot: string, file: string, kind: BadgeKind): // through path.join — badge.set used to write `/etc/passwd.json`. assertWorkspaceRelative(file); if (kind === 'folder') { - return join(workspaceRoot, BADGES_DIR, file, FOLDER_BADGE_FILENAME); + return toPosix(join(workspaceRoot, BADGES_DIR, file, FOLDER_BADGE_FILENAME)); } - return join(workspaceRoot, BADGES_DIR, `${file}.json`); + return toPosix(join(workspaceRoot, BADGES_DIR, `${file}.json`)); } export async function readBadge( @@ -58,7 +59,7 @@ export async function writeBadge( workspaceRoot, badgePath(workspaceRoot, badge.file, badge.kind), ); - await fs.mkdir(dirname(path), { recursive: true }); + await fs.mkdir(toPosix(dirname(path)), { recursive: true }); await writeMaybeNoFollow(fs, path, `${JSON.stringify(badge, null, 2)}\n`); } @@ -81,7 +82,7 @@ export async function removeBadge( * callers want listing to be robust against a single bad file. */ export async function listBadges(fs: FsLike, workspaceRoot: string): Promise { - const badgesDir = join(workspaceRoot, BADGES_DIR); + const badgesDir = toPosix(join(workspaceRoot, BADGES_DIR)); const out: BadgeFile[] = []; // Anchor containment to the WORKSPACE root, not to .bh/badges/: if // .bh/badges itself is a planted directory symlink (the whole .bh/ tree @@ -132,7 +133,7 @@ async function walk( if (!stat?.isDirectory) return; const names = await fs.readdir(dir); for (const name of names) { - const child = join(dir, name); + const child = toPosix(join(dir, name)); // Resolve canonical path + stat under a try: a hostile/broken symlink // (ELOOP on a mutual cycle, EACCES, …) skips this child rather than // crashing the listing — same robustness as the corrupt-badge skip below. diff --git a/packages/core/src/modules/badges/types.ts b/packages/core/src/modules/badges/types.ts index 19edd3b..fdc0e2e 100644 --- a/packages/core/src/modules/badges/types.ts +++ b/packages/core/src/modules/badges/types.ts @@ -117,6 +117,12 @@ export interface BadgeMarkOrphanArgs { } export type BadgeMarkOrphanResult = BadgeFile | null; +export type BadgeDeleteOrphansArgs = Record; +export interface BadgeDeleteOrphansResult { + /** Workspace-relative paths of every orphan badge that was deleted. */ + readonly deleted: readonly string[]; +} + export interface BadgeRenameArgs { readonly from: string; readonly to: string; diff --git a/packages/core/src/modules/focus/store.ts b/packages/core/src/modules/focus/store.ts index 6aceeb5..fc4622b 100644 --- a/packages/core/src/modules/focus/store.ts +++ b/packages/core/src/modules/focus/store.ts @@ -3,6 +3,7 @@ import { type FsLike, assertReadContained, assertWriteContained, + toPosix, readMaybeNoFollow, writeMaybeNoFollow, } from '../../kernel/index.js'; @@ -24,7 +25,7 @@ function oneLine(s: string): string { } export function focusPath(workspaceRoot: string): string { - return join(workspaceRoot, FOCUS_FILE); + return toPosix(join(workspaceRoot, FOCUS_FILE)); } /** @@ -196,6 +197,6 @@ export async function writeFocus( ): Promise { for (const a of active) assertFocusablePath(typeof a === 'string' ? a : a.file); const path = await assertWriteContained(fs, workspaceRoot, focusPath(workspaceRoot)); - await fs.mkdir(dirname(path), { recursive: true }); + await fs.mkdir(toPosix(dirname(path)), { recursive: true }); await writeMaybeNoFollow(fs, path, renderFocus(active, intent, source)); } diff --git a/packages/core/src/modules/inbound/store.ts b/packages/core/src/modules/inbound/store.ts index 8d0bc45..b70ce67 100644 --- a/packages/core/src/modules/inbound/store.ts +++ b/packages/core/src/modules/inbound/store.ts @@ -3,6 +3,7 @@ import { type FsLike, assertReadContained, assertWriteContained, + toPosix, readMaybeNoFollow, writeMaybeNoFollow, } from '../../kernel/index.js'; @@ -20,7 +21,7 @@ const EMPTY = (): InboundIndex => ({ }); export function inboundPath(workspaceRoot: string): string { - return join(workspaceRoot, INDEX_FILE); + return toPosix(join(workspaceRoot, INDEX_FILE)); } export async function readInbound(fs: FsLike, workspaceRoot: string): Promise { @@ -43,6 +44,6 @@ export async function writeInbound( index: InboundIndex, ): Promise { const path = await assertWriteContained(fs, workspaceRoot, inboundPath(workspaceRoot)); - await fs.mkdir(dirname(path), { recursive: true }); + await fs.mkdir(toPosix(dirname(path)), { recursive: true }); await writeMaybeNoFollow(fs, path, `${JSON.stringify(index, null, 2)}\n`); } diff --git a/packages/core/src/modules/search/commands.ts b/packages/core/src/modules/search/commands.ts index 497bcff..a57e6c5 100644 --- a/packages/core/src/modules/search/commands.ts +++ b/packages/core/src/modules/search/commands.ts @@ -1,5 +1,5 @@ import { join } from 'node:path'; -import { type Handler, canonicalize } from '../../kernel/index.js'; +import { type Handler, canonicalize, toPosix } from '../../kernel/index.js'; import type { SearchHit, SearchMatch, SearchQueryArgs, SearchQueryResult } from './types.js'; // Directories we never descend into. A superset of the renderer's NavTree @@ -170,7 +170,7 @@ export const query: Handler = async (args, c const dirs: (typeof frame)[] = []; for (const entry of listing.entries) { const childRel = frame.rel ? `${frame.rel}/${entry.name}` : entry.name; - const childAbs = join(frame.abs, entry.name); + const childAbs = toPosix(join(frame.abs, entry.name)); if (entry.type === 'dir') { // SKIP_DIRS prunes tooling/cache DIRECTORIES only — a regular FILE whose // basename happens to be `build` / `vendor` / `node_modules` (an diff --git a/packages/core/src/modules/workspace/files.ts b/packages/core/src/modules/workspace/files.ts index cf1ac18..47d8910 100644 --- a/packages/core/src/modules/workspace/files.ts +++ b/packages/core/src/modules/workspace/files.ts @@ -3,6 +3,7 @@ import { dirname, isAbsolute, join, resolve } from 'node:path'; import { type Handler, PathEscape, + toPosix, assertReadContained, assertWorkspaceRelative, assertWriteContained, @@ -46,7 +47,7 @@ export const listFiles: Handler { - const absPath = isAbsolute(args.path) ? args.path : resolve(args.path); + const absPath = toPosix(isAbsolute(args.path) ? args.path : resolve(args.path)); const stat = await ctx.fs.stat(absPath); if (!stat) { // Tagged so the desktop NavTree can render a "workspace unreachable" @@ -90,7 +91,7 @@ export const listFiles: Handler = // the request string, but a planted symlink whose NAME is innocuous still // escapes once node:fs follows it. Canonicalize and require containment, then // read the canonical path so check and open agree. (See kernel/contain.ts.) - const abs = await assertReadContained(ctx.fs, entry.path, join(entry.path, args.path)); + const abs = await assertReadContained(ctx.fs, entry.path, toPosix(join(entry.path, args.path))); // O_NOFOLLOW read closes the check-then-read TOCTOU: if the leaf is swapped // for a symlink between the guard above and this read, the open refuses it // rather than re-following. (Residual: an intermediate-component swap still @@ -206,12 +207,12 @@ export const writeFile: Handlerb,b->a cycle; EACCES; etc.) — // skip that child rather than abort the whole workspace open, mirroring @@ -179,7 +179,3 @@ async function ensureBadge( stats.created++; } -// POSIX paths on disk regardless of host (SR-v0 §3.1: relative paths use /). -function toPosix(path: string): string { - return path.split(/[\\/]/).filter(Boolean).join('/'); -} diff --git a/packages/core/src/modules/workspace/setup.ts b/packages/core/src/modules/workspace/setup.ts index 2a6acda..9731872 100644 --- a/packages/core/src/modules/workspace/setup.ts +++ b/packages/core/src/modules/workspace/setup.ts @@ -3,6 +3,7 @@ import { type FsLike, assertReadContained, assertWriteContained, + toPosix, readMaybeNoFollow, writeMaybeNoFollow, } from '../../kernel/index.js'; @@ -106,7 +107,7 @@ async function updateGitignore( fs: FsLike, workspaceRoot: string, ): Promise> { - const lexical = join(workspaceRoot, '.gitignore'); + const lexical = toPosix(join(workspaceRoot, '.gitignore')); // runSetup writes two USER files (.gitignore, CLAUDE.md) — bh's only other // write path besides the editor. A workspace "you drop in" can ship a // planted `.gitignore`/`CLAUDE.md` SYMLINK whose innocuous name escapes @@ -149,7 +150,7 @@ async function updateClaudeMd( fs: FsLike, workspaceRoot: string, ): Promise> { - const lexical = join(workspaceRoot, 'CLAUDE.md'); + const lexical = toPosix(join(workspaceRoot, 'CLAUDE.md')); try { const current = await readMaybeNoFollow( fs, diff --git a/packages/core/src/modules/workspace/store.ts b/packages/core/src/modules/workspace/store.ts index 6193280..7154ec6 100644 --- a/packages/core/src/modules/workspace/store.ts +++ b/packages/core/src/modules/workspace/store.ts @@ -1,5 +1,5 @@ import { join } from 'node:path'; -import type { FsLike } from '../../kernel/index.js'; +import { type FsLike, toPosix } from '../../kernel/index.js'; import { EMPTY_WORKSPACES, type WorkspacesFile } from './types.js'; /** @@ -8,7 +8,7 @@ import { EMPTY_WORKSPACES, type WorkspacesFile } from './types.js'; */ export function workspacesFilePath(configDir: string): string { - return join(configDir, 'workspaces.json'); + return toPosix(join(configDir, 'workspaces.json')); } export async function readWorkspaces(fs: FsLike, configDir: string): Promise { diff --git a/packages/core/test/badges.test.ts b/packages/core/test/badges.test.ts index 4bae823..3dcba77 100644 --- a/packages/core/test/badges.test.ts +++ b/packages/core/test/badges.test.ts @@ -553,3 +553,60 @@ describe('badge.rename', () => { expect(result.badge.orphan).toBeUndefined(); }); }); + +describe('badge.deleteOrphans', () => { + let ctx: TestContext; + beforeEach(async () => { + ctx = await seed(); + }); + + it('returns empty deleted list when no orphans exist', async () => { + await ctx.core.run('badge.set', { file: 'alive.md' }); + const result = (await ctx.core.run('badge.deleteOrphans', {})) as { + deleted: readonly string[]; + }; + expect(result.deleted).toEqual([]); + expect(ctx.files.has('/work/.bh/badges/alive.md.json')).toBe(true); + }); + + it('deletes all orphaned badges and returns their paths', async () => { + await ctx.core.run('badge.set', { file: 'a.md' }); + await ctx.core.run('badge.set', { file: 'b.md' }); + await ctx.core.run('badge.set', { file: 'keep.md' }); + await ctx.core.run('badge.markOrphan', { file: 'a.md' }); + await ctx.core.run('badge.markOrphan', { file: 'b.md' }); + + const result = (await ctx.core.run('badge.deleteOrphans', {})) as { + deleted: readonly string[]; + }; + expect([...result.deleted].sort()).toEqual(['a.md', 'b.md']); + expect(ctx.files.has('/work/.bh/badges/a.md.json')).toBe(false); + expect(ctx.files.has('/work/.bh/badges/b.md.json')).toBe(false); + expect(ctx.files.has('/work/.bh/badges/keep.md.json')).toBe(true); + }); + + it('leaves non-orphan badges completely untouched', async () => { + await ctx.core.run('badge.set', { + file: 'safe.md', + patch: { prompt: 'keep this' }, + }); + await ctx.core.run('badge.set', { file: 'gone.md' }); + await ctx.core.run('badge.markOrphan', { file: 'gone.md' }); + + await ctx.core.run('badge.deleteOrphans', {}); + + const safe = (await ctx.core.run('badge.get', { file: 'safe.md' })) as BadgeFile; + expect(safe.prompt).toBe('keep this'); + }); + + it('badge.list shows no orphans after deleteOrphans', async () => { + await ctx.core.run('badge.set', { file: 'x.md' }); + await ctx.core.run('badge.markOrphan', { file: 'x.md' }); + + await ctx.core.run('badge.deleteOrphans', {}); + + const list = (await ctx.core.run('badge.list', {})) as { badges: BadgeFile[] }; + expect(list.badges.some((b) => b.orphan)).toBe(false); + expect(list.badges.find((b) => b.file === 'x.md')).toBeUndefined(); + }); +}); diff --git a/packages/desktop/src/renderer/src/components/CommandPalette.tsx b/packages/desktop/src/renderer/src/components/CommandPalette.tsx index f2e1dcf..e019428 100644 --- a/packages/desktop/src/renderer/src/components/CommandPalette.tsx +++ b/packages/desktop/src/renderer/src/components/CommandPalette.tsx @@ -18,10 +18,11 @@ */ import type { SearchQueryResult } from '@basehalf/core'; -import { type CSSProperties, type JSX, useEffect, useMemo, useRef, useState } from 'react'; +import { type CSSProperties, type JSX, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { create } from 'zustand'; import { color, font, motion, radius, shadow, space, transition } from '../design.js'; import { createDemoAtDefault, promptForNewNote, tildifyPath } from '../lib/actions.js'; +import { emitBadgeChange } from '../lib/badgeBus.js'; import { highlightSegments } from '../lib/highlight.js'; import { recentFilesFor } from '../lib/recent-files.js'; import { useWorkspaceStore } from '../store/workspace.js'; @@ -194,10 +195,12 @@ export const CommandPalette = (): JSX.Element | null => { // path in the now-active workspace (the same class as the gated Search bug). const [files, setFiles] = useState([]); const [filesWorkspace, setFilesWorkspace] = useState(null); + const [orphanCount, setOrphanCount] = useState(0); useEffect(() => { if (!open) return; setFiles([]); setFilesWorkspace(null); + setOrphanCount(0); // No active workspace → no files to list (badge.list would have nothing to // resolve against). Referencing `current` here also makes it the explicit // re-fetch trigger it's meant to be. @@ -206,14 +209,17 @@ export const CommandPalette = (): JSX.Element | null => { void (async () => { try { const result = (await window.bh.run('badge.list')) as { - badges: { file: string; prompt?: string }[]; + badges: { file: string; prompt?: string; orphan?: boolean }[]; }; if (cancelled) return; + setOrphanCount(result.badges.filter((b) => b.orphan).length); setFiles( - result.badges.map((b) => ({ - file: b.file, - ...(b.prompt !== undefined && { prompt: b.prompt }), - })), + result.badges + .filter((b) => !b.orphan) + .map((b) => ({ + file: b.file, + ...(b.prompt !== undefined && { prompt: b.prompt }), + })), ); setFilesWorkspace(current); } catch { @@ -287,6 +293,21 @@ export const CommandPalette = (): JSX.Element | null => { const inputRef = useRef(null); const listRef = useRef(null); + const runDeleteOrphans = useCallback(async () => { + if (orphanCount === 0) return; + const noun = orphanCount === 1 ? 'orphaned badge' : 'orphaned badges'; + const ok = await confirm({ + title: `Delete ${orphanCount} ${noun}?`, + body: 'These badges have no backing file on disk. Their prompts and reference notes will be permanently removed. Other badges that pointed at them will keep their links (dangling refs you can clean up later).', + confirmText: 'Delete', + destructive: true, + }); + if (!ok) return; + await window.bh.run('badge.deleteOrphans', {}); + setOrphanCount(0); + emitBadgeChange(); + }, [orphanCount]); + // Build the action list from current store state + fetched files. // Memoized on (workspaces, views, files, current) so typing doesn't // rebuild — only filtering changes per keystroke. @@ -374,10 +395,19 @@ export const CommandPalette = (): JSX.Element | null => { category: 'Action', run: () => void removeActiveWorkspace(), }); + if (orphanCount > 0) { + out.push({ + id: 'action:delete-orphans', + label: `Clean up ${orphanCount} orphaned badge${orphanCount === 1 ? '' : 's'}`, + hint: 'Badges whose files were deleted from disk', + category: 'Action', + run: () => void runDeleteOrphans(), + }); + } } return out; - }, [workspaces, current, files, filesWorkspace, use, openInPanel, pickAndAdd]); + }, [workspaces, current, files, filesWorkspace, use, openInPanel, pickAndAdd, orphanCount, runDeleteOrphans]); // Filter actions by query (case-insensitive substring match on label, // hint, or category). Keeps it dead simple — no fuzzy distance yet. From 3340ebcd49da47012156e444eeef0ae50ec8ce05 Mon Sep 17 00:00:00 2001 From: Araadhay Rai Bawa Date: Sat, 6 Jun 2026 13:26:24 +0530 Subject: [PATCH 2/2] =?UTF-8?q?fix(core):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20correct=20deleteOrphans=20focus=20removal=20and=20o?= =?UTF-8?q?utbound=20ref=20cleanup;=20skip=20symlink=20tests=20on=20Window?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit badge.deleteOrphans had three issues flagged in code review: 1. Passed `to: null` to focus.renameActiveFile, which maps the active path to the literal value null rather than removing it, corrupting focus.md. Fixed by calling focus.get first and then focus.toggleActiveFile only when the file is actually present in the active list. 2. Only cleaned up inbound entries pointing AT the orphan (other badges refs removed) but left the orphan's own outbound refs as stale backlink rows in each target's inbound index. Fixed by also iterating badge.references and calling inbound.removeRef for each outbound ref. 3. Symlink-dependent tests in path-escape.test.ts, search-symlink.test.ts, and badge-rename-symlink.test.ts fail on Windows with EPERM because creating symlinks requires Developer Mode at OS level, not a code issue. Guarded all real-fs symlink describe blocks with describe.skipIf(isWin) so the suite passes cleanly on Windows without removing coverage on Linux/macOS CI. Test result: 245 passed, 26 skipped (symlink groups on win32), 0 failed. --- packages/core/src/modules/badges/commands.ts | 22 +++++++++++++---- .../core/test/badge-rename-symlink.test.ts | 4 +++- packages/core/test/path-escape.test.ts | 24 ++++++++++--------- packages/core/test/search-symlink.test.ts | 4 +++- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/packages/core/src/modules/badges/commands.ts b/packages/core/src/modules/badges/commands.ts index b437eab..93e5c0e 100644 --- a/packages/core/src/modules/badges/commands.ts +++ b/packages/core/src/modules/badges/commands.ts @@ -488,14 +488,23 @@ export const deleteOrphans: Handler, { active: readonly string[] }>( + 'focus.get', + {}, + ); + if (focusState.active.includes(badge.file)) { + await ctx.run('focus.toggleActiveFile', { file: badge.file }); + } } catch { - // no-op — focus.renameActiveFile may not support null-to (remove-only) + // Best-effort — a PathEscape or missing focus module must never abort + // the cleanup loop. } - // Clean up inbound index entries pointing at this now-gone badge. + // Clean up inbound index entries: both inbound (other badges → this one) + // and outbound (this badge → others, which leave stale backlink rows). try { const inboundRes = await ctx.run<{ file: string }, { entries: { from: string }[] }>( 'inbound.get', @@ -504,6 +513,9 @@ export const deleteOrphans: Handler { await rm(base, { recursive: true, force: true }); }); -describe('badge.rename with a hostile focus.md (real fs)', () => { +describe.skipIf(isWin)('badge.rename with a hostile focus.md (real fs)', () => { it('still completes the rename when focus.md is a workspace-escaping symlink', async () => { await writeFile(join(ws, 'note.md'), '# Note\n'); await core.run('badge.set', { file: 'note.md', patch: {} }); diff --git a/packages/core/test/path-escape.test.ts b/packages/core/test/path-escape.test.ts index 84c4682..cdf8b7a 100644 --- a/packages/core/test/path-escape.test.ts +++ b/packages/core/test/path-escape.test.ts @@ -6,6 +6,8 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { createCore } from '../src/index.js'; import { isContained } from '../src/kernel/contain.js'; +const isWin = process.platform === 'win32'; + /** * Symlink / path-traversal workspace-escape suite — the CLASS audited after * the `shell:open-path` finding. A BaseHalf workspace is "a folder you drop @@ -57,7 +59,7 @@ describe('isContained (pure)', () => { }); }); -describe('workspace.readFile', () => { +describe.skipIf(isWin)('workspace.readFile', () => { it('refuses to read THROUGH a file-symlink that points outside the workspace', async () => { await writeFile(join(outside, 'secret.txt'), 'TOP-SECRET outside contents'); await symlink(join(outside, 'secret.txt'), join(ws, 'notes.md')); @@ -73,7 +75,7 @@ describe('workspace.readFile', () => { }); }); -describe('workspace.writeFile', () => { +describe.skipIf(isWin)('workspace.writeFile', () => { it('refuses to overwrite an outside file through an existing symlink leaf (editor save)', async () => { await writeFile(join(outside, 'victim.txt'), 'IMPORTANT USER DATA DO NOT TOUCH'); await symlink(join(outside, 'victim.txt'), join(ws, 'config.md')); @@ -100,7 +102,7 @@ describe('workspace.writeFile', () => { }); }); -describe('badges store', () => { +describe.skipIf(isWin)('badges store', () => { it('refuses badge.get through a symlinked badge JSON pointing outside', async () => { await mkdir(join(ws, '.bh/badges'), { recursive: true }); await writeFile( @@ -142,7 +144,7 @@ describe('badges store', () => { }); }); -describe('focus store', () => { +describe.skipIf(isWin)('focus store', () => { it('refuses read AND write through a symlinked .bh/focus.md, never clobbering the target', async () => { await rm(join(ws, '.bh/focus.md')); await writeFile(join(outside, 'ftarget.md'), 'EXTERNAL FOCUS DATA'); @@ -155,7 +157,7 @@ describe('focus store', () => { }); }); -describe('inbound store', () => { +describe.skipIf(isWin)('inbound store', () => { it('refuses to write through a symlinked .bh/index/inbound.json (no outside clobber)', async () => { await rm(join(ws, '.bh/index/inbound.json')); await writeFile(join(outside, 'idx.json'), 'EXTERNAL INDEX'); @@ -167,7 +169,7 @@ describe('inbound store', () => { }); }); -describe('materialize walk', () => { +describe.skipIf(isWin)('materialize walk', () => { it('does NOT materialize badges for files under a symlinked directory to outside', async () => { await writeFile(join(outside, 'secret.md'), 'SECRET'); const fresh = join(base, 'ws2'); @@ -202,7 +204,7 @@ describe('materialize walk', () => { }); }); -describe('workspace setup — symlinked CLAUDE.md / .gitignore (the missed runSetup surface)', () => { +describe.skipIf(isWin)('workspace setup — symlinked CLAUDE.md / .gitignore (the missed runSetup surface)', () => { it('refuses to write THROUGH a symlinked CLAUDE.md / .gitignore on add({setup:true})', async () => { const fresh = join(base, 'wss1'); await mkdir(fresh, { recursive: true }); @@ -232,7 +234,7 @@ describe('workspace setup — symlinked CLAUDE.md / .gitignore (the missed runSe }); }); -describe('read dangling-symlink TOCTOU + write dangling-symlink directory', () => { +describe.skipIf(isWin)('read dangling-symlink TOCTOU + write dangling-symlink directory', () => { it('refuses to read a DANGLING symlink leaf (closes the create-target race)', async () => { await symlink(join(outside, 'not-yet.txt'), join(ws, 'dang.md')); // target missing await expect(core.run('workspace.readFile', { path: 'dang.md' })).rejects.toThrow( @@ -255,7 +257,7 @@ describe('read dangling-symlink TOCTOU + write dangling-symlink directory', () = }); }); -describe('.bh metadata DIRECTORY itself a symlink', () => { +describe.skipIf(isWin)('.bh metadata DIRECTORY itself a symlink', () => { it('listBadges does not enumerate through a symlinked .bh/badges directory', async () => { const fresh = join(base, 'wsbsym'); await mkdir(join(fresh, '.bh'), { recursive: true }); @@ -272,7 +274,7 @@ describe('.bh metadata DIRECTORY itself a symlink', () => { }); }); -describe('workspace.listFiles metadata oracle', () => { +describe.skipIf(isWin)('workspace.listFiles metadata oracle', () => { it('refuses listing a symlinked-out dir and filters the escaping child from the root listing', async () => { await mkdir(join(outside, 'sekrit'), { recursive: true }); await writeFile(join(outside, 'sekrit', 'k.txt'), 'k'); @@ -292,7 +294,7 @@ describe('workspace.listFiles metadata oracle', () => { }); }); -describe('anchor ELOOP robustness — the .bh metadata dir itself a symlink CYCLE', () => { +describe.skipIf(isWin)('anchor ELOOP robustness — the .bh metadata dir itself a symlink CYCLE', () => { it('badge.list returns gracefully (no PathEscape throw) when .bh/badges is a symlink cycle', async () => { await mkdir(join(ws, '.bh'), { recursive: true }); await symlink(join(ws, '.bh/b2'), join(ws, '.bh/badges')); // badges -> b2 diff --git a/packages/core/test/search-symlink.test.ts b/packages/core/test/search-symlink.test.ts index f8dea24..f169a22 100644 --- a/packages/core/test/search-symlink.test.ts +++ b/packages/core/test/search-symlink.test.ts @@ -5,6 +5,8 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { createCore } from '../src/index.js'; import type { SearchQueryResult } from '../src/index.js'; +const isWin = process.platform === 'win32'; + /** * search.query against the REAL node:fs with a REAL in-bounds directory symlink * cycle — the one thing mockFs can't model (its realpath is identity). Proves @@ -29,7 +31,7 @@ afterEach(async () => { await rm(base, { recursive: true, force: true }); }); -describe('search.query symlink-cycle guard (real fs)', () => { +describe.skipIf(isWin)('search.query symlink-cycle guard (real fs)', () => { it('breaks an in-bounds directory-symlink cycle instead of rescanning to MAX_DIRS', async () => { await writeFile(join(ws, 'note.md'), 'findme here'); // `loop -> ws` (the root itself): in-bounds (realpath stays inside ws), so