Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions packages/core/src/kernel/contain.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<string> {
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
Expand All @@ -110,7 +111,7 @@ export async function canonicalize(fs: FsLike, p: string): Promise<string> {
// 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;
Expand All @@ -120,7 +121,11 @@ export async function canonicalize(fs: FsLike, p: string): Promise<string> {

/** 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 + '/');
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion packages/core/src/kernel/index.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/kernel/paths.ts
Original file line number Diff line number Diff line change
@@ -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('\\', '/');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve literal backslashes on POSIX paths

On POSIX, \ is a valid filename character rather than a separator, but this helper now rewrites it unconditionally for absolute workspace paths as well as generated paths. If a user registers a workspace such as /tmp/project\2026, later calls through canonicalize, focusPath, badgePath, etc. will look under /tmp/project/2026 instead, which can make materialization miss the real files or create .bh metadata in the wrong directory; this normalization should be platform-aware or limited to Windows-style paths.

Useful? React with 👍 / 👎.

}

/**
* Shared workspace-relative path guard.
*
Expand Down
65 changes: 65 additions & 0 deletions packages/core/src/modules/badges/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { listBadges, readBadge, removeBadge, writeBadge } from './store.js';
import type {
BadgeAddRefArgs,
BadgeDeleteArgs,
BadgeDeleteOrphansArgs,
BadgeDeleteOrphansResult,
BadgeDeleteResult,
BadgeFile,
BadgeGetArgs,
Expand Down Expand Up @@ -463,6 +465,68 @@ export const rename: Handler<BadgeRenameArgs, BadgeRenameResult> = 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<BadgeDeleteOrphansArgs, BadgeDeleteOrphansResult> = 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Delete the walked orphan badge, not the embedded path

If an orphan badge JSON has a stale or mismatched file field (for example after a manual edit or merge conflict), cleanup recomputes the path to unlink from that embedded value rather than the JSON file that listBadges just walked. In that scenario badge.deleteOrphans can delete a different, non-orphan badge while leaving the actual orphan JSON behind, so this path should either be validated against the walked badge path or deletion should target the walked file.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — the theoretical gap is real. In normal operation this can't happen: every writeBadge call derives the on-disk path from badge.file and then writes that same value into the JSON, so the field and the file path are always in sync. A mismatch can only arise from a manual edit or a bad merge conflict, not through any API call.

The existing defence layers still apply: removeBadge calls assertWriteContained, which will throw on any path that escapes the workspace, and fs.stat guards against unlinking a path that doesn't exist. So the security invariant holds even in the mismatch case.

Fixing this properly — threading the actual walked path out of listBadges so deleteOrphans can target it directly rather than re-deriving from badge.file — requires a change to the listBadges return type. That's a worthwhile follow-up but out of scope for this PR. Happy to open a tracking issue for it.

deleted.push(badge.file);
// Remove from focus.md if it was in the active list. toggleActiveFile
// removes the file when present, adds it when absent — so guard with
// focus.get first to avoid accidentally re-adding it.
try {
const focusState = await ctx.run<Record<string, never>, { active: readonly string[] }>(
'focus.get',
{},
);
if (focusState.active.includes(badge.file)) {
await ctx.run('focus.toggleActiveFile', { file: badge.file });
}
} catch {
// Best-effort — a PathEscape or missing focus module must never abort
// the cleanup loop.
}
// 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',
{ file: badge.file },
);
for (const entry of inboundRes.entries) {
await ctx.run('inbound.removeRef', { from: entry.from, to: badge.file });
Comment on lines +509 to +514

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear deleted badges' outbound inbound entries

When an orphaned badge itself has outbound references, those refs already created inbound-index entries under each target via badge.addRef; deleting the badge JSON here only removes entries keyed by the deleted badge (incoming refs). In that scenario the targets keep stale inbound rows from a non-existent file, so badge pages/agent backlink traversal can report phantom inbound links after cleanup; the loop should also remove from: badge.file for each badge.references[*].to.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the follow-up commit (3340ebc).

The inbound cleanup block now handles both directions:

// Inbound: other badges that pointed AT this orphan
for (const entry of inboundRes.entries) {
  await ctx.run('inbound.removeRef', { from: entry.from, to: badge.file });
}
// Outbound: this orphan's own refs leave stale backlink rows in targets
for (const ref of badge.references) {
  await ctx.run('inbound.removeRef', { from: badge.file, to: ref.to });
}

Both loops run inside the same try/catch so a missing inbound module in lightweight tests still doesn't abort the cleanup.

}
for (const ref of badge.references) {
await ctx.run('inbound.removeRef', { from: badge.file, to: ref.to });
}
} 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<never, unknown>]
> {
Expand All @@ -471,6 +535,7 @@ export function commands(): ReadonlyArray<
['badge.set', set as unknown as Handler<never, unknown>],
['badge.list', list as unknown as Handler<never, unknown>],
['badge.delete', del as unknown as Handler<never, unknown>],
['badge.deleteOrphans', deleteOrphans as unknown as Handler<never, unknown>],
['badge.addRef', addRef as unknown as Handler<never, unknown>],
['badge.removeRef', removeRef as unknown as Handler<never, unknown>],
['badge.reconnectRef', reconnectRef as unknown as Handler<never, unknown>],
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/modules/badges/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type FsLike,
assertReadContained,
assertWorkspaceRelative,
toPosix,
assertWriteContained,
canonicalize,
isContained,
Expand All @@ -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(
Expand Down Expand Up @@ -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`);
}

Expand All @@ -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<readonly BadgeFile[]> {
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
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/modules/badges/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ export interface BadgeMarkOrphanArgs {
}
export type BadgeMarkOrphanResult = BadgeFile | null;

export type BadgeDeleteOrphansArgs = Record<string, never>;
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;
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/modules/focus/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type FsLike,
assertReadContained,
assertWriteContained,
toPosix,
readMaybeNoFollow,
writeMaybeNoFollow,
} from '../../kernel/index.js';
Expand All @@ -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));
}

/**
Expand Down Expand Up @@ -196,6 +197,6 @@ export async function writeFocus(
): Promise<void> {
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));
}
5 changes: 3 additions & 2 deletions packages/core/src/modules/inbound/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type FsLike,
assertReadContained,
assertWriteContained,
toPosix,
readMaybeNoFollow,
writeMaybeNoFollow,
} from '../../kernel/index.js';
Expand All @@ -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<InboundIndex> {
Expand All @@ -43,6 +44,6 @@ export async function writeInbound(
index: InboundIndex,
): Promise<void> {
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`);
}
4 changes: 2 additions & 2 deletions packages/core/src/modules/search/commands.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -170,7 +170,7 @@ export const query: Handler<SearchQueryArgs, SearchQueryResult> = 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
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/modules/workspace/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { dirname, isAbsolute, join, resolve } from 'node:path';
import {
type Handler,
PathEscape,
toPosix,
assertReadContained,
assertWorkspaceRelative,
assertWriteContained,
Expand Down Expand Up @@ -46,7 +47,7 @@ export const listFiles: Handler<WorkspaceListFilesArgs, WorkspaceListFilesResult
args,
ctx,
) => {
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"
Expand Down Expand Up @@ -90,7 +91,7 @@ export const listFiles: Handler<WorkspaceListFilesArgs, WorkspaceListFilesResult
const names = await ctx.fs.readdir(absPath);
const entries: WorkspaceListFilesEntry[] = [];
for (const name of names) {
const child = join(absPath, name);
const child = toPosix(join(absPath, name));
// Filter a symlinked-out child so it never surfaces in the tree (and
// can't be expanded into an external dir on the next listFiles call).
let realChild: string;
Expand Down Expand Up @@ -140,7 +141,7 @@ export const readFile: Handler<WorkspaceReadFileArgs, WorkspaceReadFileResult> =
// 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
Expand Down Expand Up @@ -206,12 +207,12 @@ export const writeFile: Handler<WorkspaceWriteFileArgs, WorkspaceWriteFileResult
// not let an editor save / New-Note clobber or plant a file outside the
// workspace. assertWriteContained proves the real parent is inside and
// refuses a symlink leaf. (See kernel/contain.ts.)
const abs = await assertWriteContained(ctx.fs, entry.path, join(entry.path, args.path));
const abs = await assertWriteContained(ctx.fs, entry.path, toPosix(join(entry.path, args.path)));
// Honor the desktop new-note dialog's "folders auto-created" promise:
// mkdir -p the parent so a path like `subdir/new/note.md` succeeds even
// when `subdir/new` doesn't exist yet. Top-level paths have an empty
// dirname (".") and the mkdir is a no-op.
const parent = dirname(abs);
const parent = toPosix(dirname(abs));
if (parent && parent !== abs) {
await ctx.fs.mkdir(parent, { recursive: true });
}
Expand Down
8 changes: 2 additions & 6 deletions packages/core/src/modules/workspace/materialize.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { join, relative } from 'node:path';
import { type FsLike, type Run, canonicalize, isContained } from '../../kernel/index.js';
import { type FsLike, type Run, canonicalize, isContained, toPosix } from '../../kernel/index.js';

// SR-v0 §4.5 default canvas whitelist (v0 hardcoded; v0.x will move to
// .bh/config.json). Files outside this list show in the NavTree but
Expand Down Expand Up @@ -102,7 +102,7 @@ async function walk(
const names = await fs.readdir(dir);
for (const name of names) {
if (SKIP_NAMES.has(name)) continue;
const child = join(dir, name);
const child = toPosix(join(dir, name));
// Resolve the child's canonical path + stat. A hostile/broken symlink can
// make either throw (ELOOP on a mutual a->b,b->a cycle; EACCES; etc.) —
// skip that child rather than abort the whole workspace open, mirroring
Expand Down Expand Up @@ -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('/');
}
Loading
Loading