feat(session): Explicit session id naming (#5076)

packages/coding-agent/test/args.test.ts

@@ -130,6 +130,11 @@ describe("parseArgs", () => {
 			expect(result.session).toBe("/path/to/session.jsonl");
 		});
 
+		test("parses --session-id", () => {
+			const result = parseArgs(["--session-id", "orchestrated-session"]);
+			expect(result.sessionId).toBe("orchestrated-session");
+		});
+
 		test("parses --fork", () => {
 			const result = parseArgs(["--fork", "1234abcd"]);
 			expect(result.fork).toBe("1234abcd");

packages/coding-agent/test/session-id-readonly.test.ts

@@ -0,0 +1,131 @@
+import { spawn } from "node:child_process";
+import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import { ENV_AGENT_DIR } from "../src/config.ts";
+
+const cliPath = resolve(__dirname, "../src/cli.ts");
+const tempDirs: string[] = [];
+
+afterEach(() => {
+	for (const dir of tempDirs.splice(0)) {
+		rmSync(dir, { recursive: true, force: true });
+	}
+});
+
+function createTempDir(): string {
+	const dir = mkdtempSync(join(tmpdir(), "pi-session-id-readonly-"));
+	tempDirs.push(dir);
+	return dir;
+}
+
+function hasSessionWithId(root: string, sessionId: string): boolean {
+	if (!existsSync(root)) return false;
+	for (const entry of readdirSync(root, { withFileTypes: true })) {
+		const path = join(root, entry.name);
+		if (entry.isDirectory() && hasSessionWithId(path, sessionId)) return true;
+		if (!entry.isFile() || !entry.name.endsWith(".jsonl")) continue;
+
+		try {
+			const firstLine = readFileSync(path, "utf8").split("\n", 1)[0];
+			const header = JSON.parse(firstLine) as { type?: string; id?: string };
+			if (header.type === "session" && header.id === sessionId) return true;
+		} catch {
+			// Ignore malformed session files.
+		}
+	}
+	return false;
+}
+
+interface CliDirs {
+	agentDir: string;
+	projectDir: string;
+	sessionDir: string;
+}
+
+async function runCli(
+	args: string[] | ((dirs: CliDirs) => string[]),
+	setup?: (dirs: CliDirs) => void,
+): Promise<{ code: number | null; agentDir: string; stderr: string }> {
+	const tempRoot = createTempDir();
+	const dirs: CliDirs = {
+		agentDir: join(tempRoot, "agent"),
+		projectDir: join(tempRoot, "project"),
+		sessionDir: join(tempRoot, "sessions"),
+	};
+	mkdirSync(dirs.agentDir, { recursive: true });
+	mkdirSync(dirs.projectDir, { recursive: true });
+	setup?.(dirs);
+	const resolvedArgs = typeof args === "function" ? args(dirs) : args;
+
+	let stderr = "";
+	const code = await new Promise<number | null>((resolvePromise, reject) => {
+		const child = spawn(process.execPath, [cliPath, ...resolvedArgs], {
+			cwd: dirs.projectDir,
+			env: {
+				...process.env,
+				[ENV_AGENT_DIR]: dirs.agentDir,
+				PI_OFFLINE: "1",
+				TSX_TSCONFIG_PATH: resolve(__dirname, "../../../tsconfig.json"),
+			},
+			stdio: ["ignore", "ignore", "pipe"],
+		});
+		child.stderr.on("data", (chunk) => {
+			stderr += chunk.toString();
+		});
+		child.on("error", reject);
+		child.on("close", resolvePromise);
+	});
+
+	return { code, agentDir: dirs.agentDir, stderr };
+}
+
+function writeSession(sessionDir: string, cwd: string, id: string): void {
+	writeFileSync(
+		join(sessionDir, `${id}.jsonl`),
+		`${JSON.stringify({ type: "session", version: 3, id, timestamp: new Date().toISOString(), cwd })}\n`,
+	);
+}
+
+describe("--session-id read-only commands", () => {
+	it("does not reserve a session for --help", async () => {
+		const result = await runCli(["--session-id", "read-only-help", "--help"]);
+
+		expect(result.code).toBe(0);
+		expect(hasSessionWithId(join(result.agentDir, "sessions"), "read-only-help")).toBe(false);
+	});
+
+	it("does not reserve a session for --list-models", async () => {
+		const result = await runCli(["--session-id", "read-only-models", "--list-models"]);
+
+		expect(result.code).toBe(0);
+		expect(hasSessionWithId(join(result.agentDir, "sessions"), "read-only-models")).toBe(false);
+	});
+
+	it("rejects an existing fork target session id", async () => {
+		const result = await runCli(
+			(dirs) => ["--session-dir", dirs.sessionDir, "--fork", "source-id", "--session-id", "existing-id", "-p", "hi"],
+			(dirs) => {
+				mkdirSync(dirs.sessionDir, { recursive: true });
+				writeSession(dirs.sessionDir, dirs.projectDir, "source-id");
+				writeSession(dirs.sessionDir, dirs.projectDir, "existing-id");
+			},
+		);
+
+		expect(result.code).toBe(1);
+		expect(result.stderr).toContain("Session already exists with id 'existing-id'");
+	});
+});
+
+describe("--session-id validation", () => {
+	it("rejects ids invalid under SessionManager rules without stack traces", async () => {
+		for (const id of ["-bad", "bad id"]) {
+			const result = await runCli(["--session-id", id, "-p", "hi"]);
+
+			expect(result.code).toBe(1);
+			expect(result.stderr).toContain("Session id must be non-empty");
+			expect(result.stderr).not.toContain("SessionManager.create");
+		}
+	});
+});

packages/coding-agent/test/session-manager/custom-session-id.test.ts

@@ -1,6 +1,6 @@
-import { mkdtempSync, writeFileSync } from "node:fs";
+import { existsSync, mkdtempSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
-import { join } from "node:path";
+import { basename, join } from "node:path";
 import { describe, expect, it } from "vitest";
 import { SessionManager } from "../../src/core/session-manager.ts";
 
@@ -13,6 +13,23 @@ describe("SessionManager.newSession with custom id", () => {
 		expect(session.getSessionId()).toBe("my-custom-id");
 	});
 
+	it("allows alphanumeric session ids with interior punctuation", () => {
+		const session = SessionManager.inMemory();
+		session.newSession({ id: "abc-123_def.456" });
+		expect(session.getSessionId()).toBe("abc-123_def.456");
+	});
+
+	it("rejects invalid custom session ids", () => {
+		const invalidIds = ["", "-abc", "abc-", "_abc", "abc_", ".abc", "abc.", "abc/def", "abc\\def", "abc def"];
+
+		for (const id of invalidIds) {
+			const session = SessionManager.inMemory();
+			expect(() => session.newSession({ id })).toThrow(
+				"Session id must be non-empty, contain only alphanumeric characters",
+			);
+		}
+	});
+
 	it("generates a UUIDv7 id when no id is provided", () => {
 		const session = SessionManager.inMemory();
 		session.newSession();
@@ -46,6 +63,18 @@ describe("SessionManager.newSession with custom id", () => {
 		expect(session.getHeader()!.id).toBe(session.getSessionId());
 	});
 
+	it("uses the provided id when creating a persisted session", () => {
+		const tempDir = mkdtempSync(join(tmpdir(), "pi-session-manager-"));
+		const session = SessionManager.create(tempDir, tempDir, { id: "created-session-id" });
+
+		expect(session.getSessionId()).toBe("created-session-id");
+		expect(session.getHeader()!.id).toBe("created-session-id");
+		const sessionFile = session.getSessionFile()!;
+		expect(sessionFile).toContain("created-session-id");
+		expect(basename(sessionFile)).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z_created-session-id\.jsonl$/);
+		expect(existsSync(sessionFile)).toBe(false);
+	});
+
 	it("generates a UUIDv7 id when creating a branched session", () => {
 		const session = SessionManager.inMemory();
 		const firstId = session.appendMessage({
@@ -106,4 +135,28 @@ describe("SessionManager.newSession with custom id", () => {
 		expect(header!.id).toMatch(UUID_V7_RE);
 		expect(header!.parentSession).toBe(sourcePath);
 	});
+
+	it("uses the provided id when forking from another session file", () => {
+		const tempDir = mkdtempSync(join(tmpdir(), "pi-session-manager-"));
+		const sourcePath = join(tempDir, "source.jsonl");
+		writeFileSync(
+			sourcePath,
+			`${JSON.stringify({
+				type: "session",
+				version: 3,
+				id: "source-session-id",
+				timestamp: new Date().toISOString(),
+				cwd: tempDir,
+			})}\n`,
+		);
+
+		const forked = SessionManager.forkFrom(sourcePath, tempDir, tempDir, { id: "forked-session-id" });
+		const header = forked.getHeader();
+		expect(header).not.toBeNull();
+		expect(header!.id).toBe("forked-session-id");
+		expect(header!.parentSession).toBe(sourcePath);
+		const sessionFile = forked.getSessionFile()!;
+		expect(sessionFile).toContain("forked-session-id");
+		expect(basename(sessionFile)).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z_forked-session-id\.jsonl$/);
+	});
 });