fix(coding-agent): handle symlinked session selector paths closes #3364
This commit is contained in:
@@ -8,6 +8,7 @@
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
- Fixed threaded `/resume` session relationships and current-session detection to canonicalize symlinked session paths during selector comparisons, so shared session directories no longer break parent-child matching or active-session delete protection ([#3364](https://github.com/badlogic/pi-mono/issues/3364))
|
||||||
- Fixed `/session`, Sessions docs, and CLI help to consistently document that session reuse supports both file paths and session IDs, and that `/session` shows the current session ID ([#3390](https://github.com/badlogic/pi-mono/issues/3390))
|
- Fixed `/session`, Sessions docs, and CLI help to consistently document that session reuse supports both file paths and session IDs, and that `/session` shows the current session ID ([#3390](https://github.com/badlogic/pi-mono/issues/3390))
|
||||||
- Fixed Windows pnpm global install detection to recognize `\\.pnpm\\` store paths, so update notices now suggest `pnpm install -g @mariozechner/pi-coding-agent` instead of falling back to npm ([#3378](https://github.com/badlogic/pi-mono/issues/3378))
|
- Fixed Windows pnpm global install detection to recognize `\\.pnpm\\` store paths, so update notices now suggest `pnpm install -g @mariozechner/pi-coding-agent` instead of falling back to npm ([#3378](https://github.com/badlogic/pi-mono/issues/3378))
|
||||||
- Fixed missing `@sinclair/typebox` runtime dependency in `@mariozechner/pi-coding-agent`, so strict pnpm installs no longer fail with `ERR_MODULE_NOT_FOUND` when starting `pi` ([#3434](https://github.com/badlogic/pi-mono/issues/3434))
|
- Fixed missing `@sinclair/typebox` runtime dependency in `@mariozechner/pi-coding-agent`, so strict pnpm installs no longer fail with `ERR_MODULE_NOT_FOUND` when starting `pi` ([#3434](https://github.com/badlogic/pi-mono/issues/3434))
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { spawnSync } from "node:child_process";
|
import { spawnSync } from "node:child_process";
|
||||||
import { existsSync } from "node:fs";
|
import { existsSync, realpathSync } from "node:fs";
|
||||||
import { unlink } from "node:fs/promises";
|
import { unlink } from "node:fs/promises";
|
||||||
import * as os from "node:os";
|
import * as os from "node:os";
|
||||||
import {
|
import {
|
||||||
@@ -47,6 +47,15 @@ function formatSessionDate(date: Date): string {
|
|||||||
return `${Math.floor(diffDays / 365)}y`;
|
return `${Math.floor(diffDays / 365)}y`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function canonicalizePath(path: string | undefined): string | undefined {
|
||||||
|
if (!path) return path;
|
||||||
|
try {
|
||||||
|
return realpathSync(path);
|
||||||
|
} catch {
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
class SessionSelectorHeader implements Component {
|
class SessionSelectorHeader implements Component {
|
||||||
private scope: SessionScope;
|
private scope: SessionScope;
|
||||||
private sortMode: SortMode;
|
private sortMode: SortMode;
|
||||||
@@ -203,14 +212,16 @@ function buildSessionTree(sessions: SessionInfo[]): SessionTreeNode[] {
|
|||||||
const byPath = new Map<string, SessionTreeNode>();
|
const byPath = new Map<string, SessionTreeNode>();
|
||||||
|
|
||||||
for (const session of sessions) {
|
for (const session of sessions) {
|
||||||
byPath.set(session.path, { session, children: [] });
|
const sessionPath = canonicalizePath(session.path) ?? session.path;
|
||||||
|
byPath.set(sessionPath, { session, children: [] });
|
||||||
}
|
}
|
||||||
|
|
||||||
const roots: SessionTreeNode[] = [];
|
const roots: SessionTreeNode[] = [];
|
||||||
|
|
||||||
for (const session of sessions) {
|
for (const session of sessions) {
|
||||||
const node = byPath.get(session.path)!;
|
const sessionPath = canonicalizePath(session.path) ?? session.path;
|
||||||
const parentPath = session.parentSessionPath;
|
const node = byPath.get(sessionPath)!;
|
||||||
|
const parentPath = canonicalizePath(session.parentSessionPath);
|
||||||
|
|
||||||
if (parentPath && byPath.has(parentPath)) {
|
if (parentPath && byPath.has(parentPath)) {
|
||||||
byPath.get(parentPath)!.children.push(node);
|
byPath.get(parentPath)!.children.push(node);
|
||||||
@@ -273,7 +284,7 @@ class SessionList implements Component, Focusable {
|
|||||||
private keybindings: KeybindingsManager;
|
private keybindings: KeybindingsManager;
|
||||||
private showPath = false;
|
private showPath = false;
|
||||||
private confirmingDeletePath: string | null = null;
|
private confirmingDeletePath: string | null = null;
|
||||||
private currentSessionFilePath?: string;
|
private currentSessionCanonicalPath?: string;
|
||||||
public onSelect?: (sessionPath: string) => void;
|
public onSelect?: (sessionPath: string) => void;
|
||||||
public onCancel?: () => void;
|
public onCancel?: () => void;
|
||||||
public onExit: () => void = () => {};
|
public onExit: () => void = () => {};
|
||||||
@@ -312,7 +323,7 @@ class SessionList implements Component, Focusable {
|
|||||||
this.sortMode = sortMode;
|
this.sortMode = sortMode;
|
||||||
this.nameFilter = nameFilter;
|
this.nameFilter = nameFilter;
|
||||||
this.keybindings = keybindings;
|
this.keybindings = keybindings;
|
||||||
this.currentSessionFilePath = currentSessionFilePath;
|
this.currentSessionCanonicalPath = canonicalizePath(currentSessionFilePath);
|
||||||
this.filterSessions("");
|
this.filterSessions("");
|
||||||
|
|
||||||
// Handle Enter in search input - select current item
|
// Handle Enter in search input - select current item
|
||||||
@@ -374,7 +385,7 @@ class SessionList implements Component, Focusable {
|
|||||||
if (!selected) return;
|
if (!selected) return;
|
||||||
|
|
||||||
// Prevent deleting current session
|
// Prevent deleting current session
|
||||||
if (this.currentSessionFilePath && selected.session.path === this.currentSessionFilePath) {
|
if (this.isCurrentSessionPath(selected.session.path)) {
|
||||||
this.onError?.("Cannot delete the currently active session");
|
this.onError?.("Cannot delete the currently active session");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -382,6 +393,11 @@ class SessionList implements Component, Focusable {
|
|||||||
this.setConfirmingDeletePath(selected.session.path);
|
this.setConfirmingDeletePath(selected.session.path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private isCurrentSessionPath(path: string): boolean {
|
||||||
|
if (!this.currentSessionCanonicalPath) return false;
|
||||||
|
return (canonicalizePath(path) ?? path) === this.currentSessionCanonicalPath;
|
||||||
|
}
|
||||||
|
|
||||||
invalidate(): void {}
|
invalidate(): void {}
|
||||||
|
|
||||||
render(width: number): string[] {
|
render(width: number): string[] {
|
||||||
@@ -424,7 +440,7 @@ class SessionList implements Component, Focusable {
|
|||||||
const session = node.session;
|
const session = node.session;
|
||||||
const isSelected = i === this.selectedIndex;
|
const isSelected = i === this.selectedIndex;
|
||||||
const isConfirmingDelete = session.path === this.confirmingDeletePath;
|
const isConfirmingDelete = session.path === this.confirmingDeletePath;
|
||||||
const isCurrent = this.currentSessionFilePath === session.path;
|
const isCurrent = this.isCurrentSessionPath(session.path);
|
||||||
|
|
||||||
// Build tree prefix
|
// Build tree prefix
|
||||||
const prefix = this.buildTreePrefix(node);
|
const prefix = this.buildTreePrefix(node);
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
|
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join } from "node:path";
|
||||||
import { setKeybindings } from "@mariozechner/pi-tui";
|
import { setKeybindings } from "@mariozechner/pi-tui";
|
||||||
import { beforeAll, beforeEach, describe, expect, it } from "vitest";
|
import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
|
||||||
import { KeybindingsManager } from "../src/core/keybindings.js";
|
import { KeybindingsManager } from "../src/core/keybindings.js";
|
||||||
import type { SessionInfo } from "../src/core/session-manager.js";
|
import type { SessionInfo } from "../src/core/session-manager.js";
|
||||||
import { SessionSelectorComponent } from "../src/modes/interactive/components/session-selector.js";
|
import { SessionSelectorComponent } from "../src/modes/interactive/components/session-selector.js";
|
||||||
@@ -27,12 +30,17 @@ async function flushPromises(): Promise<void> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function stripAnsi(text: string): string {
|
||||||
|
return text.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, "");
|
||||||
|
}
|
||||||
|
|
||||||
function makeSession(overrides: Partial<SessionInfo> & { id: string }): SessionInfo {
|
function makeSession(overrides: Partial<SessionInfo> & { id: string }): SessionInfo {
|
||||||
return {
|
return {
|
||||||
path: overrides.path ?? `/tmp/${overrides.id}.jsonl`,
|
path: overrides.path ?? `/tmp/${overrides.id}.jsonl`,
|
||||||
id: overrides.id,
|
id: overrides.id,
|
||||||
cwd: overrides.cwd ?? "",
|
cwd: overrides.cwd ?? "",
|
||||||
name: overrides.name,
|
name: overrides.name,
|
||||||
|
parentSessionPath: overrides.parentSessionPath,
|
||||||
created: overrides.created ?? new Date(0),
|
created: overrides.created ?? new Date(0),
|
||||||
modified: overrides.modified ?? new Date(0),
|
modified: overrides.modified ?? new Date(0),
|
||||||
messageCount: overrides.messageCount ?? 1,
|
messageCount: overrides.messageCount ?? 1,
|
||||||
@@ -41,11 +49,52 @@ function makeSession(overrides: Partial<SessionInfo> & { id: string }): SessionI
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function createSymlinkedSessionPaths(): {
|
||||||
|
baseDir: string;
|
||||||
|
parentAliasA: string;
|
||||||
|
parentAliasB: string;
|
||||||
|
childAliasB: string;
|
||||||
|
} {
|
||||||
|
const baseDir = mkdtempSync(join(tmpdir(), "pi-session-selector-"));
|
||||||
|
const realDir = join(baseDir, "real");
|
||||||
|
const aliasADir = join(baseDir, "alias-a");
|
||||||
|
const aliasBDir = join(baseDir, "alias-b");
|
||||||
|
mkdirSync(realDir, { recursive: true });
|
||||||
|
mkdirSync(aliasADir, { recursive: true });
|
||||||
|
mkdirSync(aliasBDir, { recursive: true });
|
||||||
|
|
||||||
|
const sharedDir = join(realDir, "sessions");
|
||||||
|
mkdirSync(sharedDir, { recursive: true });
|
||||||
|
const aliasASessions = join(aliasADir, "sessions");
|
||||||
|
const aliasBSessions = join(aliasBDir, "sessions");
|
||||||
|
symlinkSync(sharedDir, aliasASessions);
|
||||||
|
symlinkSync(sharedDir, aliasBSessions);
|
||||||
|
|
||||||
|
const parentRealPath = join(sharedDir, "parent.jsonl");
|
||||||
|
const childRealPath = join(sharedDir, "child.jsonl");
|
||||||
|
writeFileSync(parentRealPath, "parent\n");
|
||||||
|
writeFileSync(childRealPath, "child\n");
|
||||||
|
|
||||||
|
return {
|
||||||
|
baseDir,
|
||||||
|
parentAliasA: join(aliasASessions, "parent.jsonl"),
|
||||||
|
parentAliasB: join(aliasBSessions, "parent.jsonl"),
|
||||||
|
childAliasB: join(aliasBSessions, "child.jsonl"),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
const CTRL_D = "\x04";
|
const CTRL_D = "\x04";
|
||||||
const CTRL_BACKSPACE = "\x1b[127;5u";
|
const CTRL_BACKSPACE = "\x1b[127;5u";
|
||||||
|
|
||||||
describe("session selector path/delete interactions", () => {
|
describe("session selector path/delete interactions", () => {
|
||||||
const keybindings = new KeybindingsManager();
|
const keybindings = new KeybindingsManager();
|
||||||
|
const tempDirs: string[] = [];
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
for (const dir of tempDirs.splice(0)) {
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
// Ensure test isolation: keybindings are a global singleton
|
// Ensure test isolation: keybindings are a global singleton
|
||||||
@@ -196,4 +245,71 @@ describe("session selector path/delete interactions", () => {
|
|||||||
allDeferred.resolve([makeSession({ id: "all" })]);
|
allDeferred.resolve([makeSession({ id: "all" })]);
|
||||||
await flushPromises();
|
await flushPromises();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("threads sessions when parent and child paths use different symlink aliases", async () => {
|
||||||
|
const paths = createSymlinkedSessionPaths();
|
||||||
|
tempDirs.push(paths.baseDir);
|
||||||
|
|
||||||
|
const sessions = [
|
||||||
|
makeSession({
|
||||||
|
id: "parent",
|
||||||
|
path: paths.parentAliasB,
|
||||||
|
name: "Parent",
|
||||||
|
modified: new Date("2026-01-01T00:00:00.000Z"),
|
||||||
|
}),
|
||||||
|
makeSession({
|
||||||
|
id: "child",
|
||||||
|
path: paths.childAliasB,
|
||||||
|
parentSessionPath: paths.parentAliasA,
|
||||||
|
name: "Child",
|
||||||
|
modified: new Date("2025-12-31T00:00:00.000Z"),
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
const selector = new SessionSelectorComponent(
|
||||||
|
async () => sessions,
|
||||||
|
async () => [],
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
{ keybindings },
|
||||||
|
);
|
||||||
|
await flushPromises();
|
||||||
|
|
||||||
|
const output = stripAnsi(selector.render(120).join("\n"));
|
||||||
|
expect(output).toContain("Parent");
|
||||||
|
expect(output).toContain("└─ Child");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("treats the current session as active across symlink aliases", async () => {
|
||||||
|
const paths = createSymlinkedSessionPaths();
|
||||||
|
tempDirs.push(paths.baseDir);
|
||||||
|
|
||||||
|
const sessions = [makeSession({ id: "parent", path: paths.parentAliasB, name: "Parent" })];
|
||||||
|
const selector = new SessionSelectorComponent(
|
||||||
|
async () => sessions,
|
||||||
|
async () => [],
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
() => {},
|
||||||
|
{ keybindings },
|
||||||
|
paths.parentAliasA,
|
||||||
|
);
|
||||||
|
await flushPromises();
|
||||||
|
|
||||||
|
const list = selector.getSessionList();
|
||||||
|
const confirmationChanges: Array<string | null> = [];
|
||||||
|
let errorMessage: string | undefined;
|
||||||
|
list.onDeleteConfirmationChange = (path) => confirmationChanges.push(path);
|
||||||
|
list.onError = (message) => {
|
||||||
|
errorMessage = message;
|
||||||
|
};
|
||||||
|
|
||||||
|
list.handleInput(CTRL_D);
|
||||||
|
|
||||||
|
expect(confirmationChanges).toEqual([]);
|
||||||
|
expect(errorMessage).toBe("Cannot delete the currently active session");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user