From 119e17d00122c366de2ead5ce5b2744289756045 Mon Sep 17 00:00:00 2001 From: HiranoMasaaki Date: Sun, 15 Feb 2026 06:38:02 +0000 Subject: [PATCH] refactor: separate lockfile I/O from runtime public API Remove Node.js-specific lockfile I/O (findLockfile, loadLockfile) from @perstack/runtime to make it isomorphic-ready. Lockfile search/read logic moved to @perstack/tui (Node.js-only) and inlined in bin/cli.ts. Co-Authored-By: Claude Opus 4.6 --- apps/runtime/bin/cli.ts | 21 +- apps/runtime/src/helpers/index.ts | 6 +- apps/runtime/src/helpers/lockfile.test.ts | 270 +++++----------------- apps/runtime/src/helpers/lockfile.ts | 50 +--- apps/runtime/src/index.ts | 2 +- packages/tui/src/lockfile.test.ts | 162 +++++++++++++ packages/tui/src/lockfile.ts | 44 ++++ packages/tui/src/run-handler.ts | 3 +- packages/tui/src/start-handler.ts | 3 +- 9 files changed, 286 insertions(+), 275 deletions(-) create mode 100644 packages/tui/src/lockfile.test.ts create mode 100644 packages/tui/src/lockfile.ts diff --git a/apps/runtime/bin/cli.ts b/apps/runtime/bin/cli.ts index 8c88330f..818ca87b 100755 --- a/apps/runtime/bin/cli.ts +++ b/apps/runtime/bin/cli.ts @@ -1,16 +1,20 @@ #!/usr/bin/env node +import { readFileSync } from "node:fs" +import path from "node:path" import type { Checkpoint, RunEvent, RuntimeEvent } from "@perstack/core" import { createFilteredEventListener, + type Lockfile, + lockfileSchema, parseWithFriendlyError, runCommandInputSchema, validateEventFilter, } from "@perstack/core" import { Command } from "commander" +import TOML from "smol-toml" import pkg from "../package.json" with { type: "json" } import { resolveRunContext } from "../src/cli/context.js" -import { findLockfile, loadLockfile } from "../src/helpers/lockfile.js" import { run } from "../src/run.js" const defaultEventListener = (event: RunEvent | RuntimeEvent) => console.log(JSON.stringify(event)) @@ -93,8 +97,19 @@ program model: input.options.model, envPath: input.options.envPath, }) - const lockfilePath = findLockfile(input.options.config) - const lockfile = lockfilePath ? (loadLockfile(lockfilePath) ?? undefined) : undefined + let lockfile: Lockfile | undefined + try { + const lockfilePath = input.options.config + ? path.join( + path.dirname(path.resolve(process.cwd(), input.options.config)), + "perstack.lock", + ) + : path.resolve(process.cwd(), "perstack.lock") + const content = readFileSync(lockfilePath, "utf-8") + lockfile = parseWithFriendlyError(lockfileSchema, TOML.parse(content), "perstack.lock") + } catch { + // No lockfile found or invalid - continue without it + } await run( { setting: { diff --git a/apps/runtime/src/helpers/index.ts b/apps/runtime/src/helpers/index.ts index 91d1f847..09b13ca8 100644 --- a/apps/runtime/src/helpers/index.ts +++ b/apps/runtime/src/helpers/index.ts @@ -4,11 +4,7 @@ export { createNextStepCheckpoint, type DelegationStateResult, } from "./checkpoint.js" -export { - findLockfile, - getLockfileExpertToolDefinitions, - loadLockfile, -} from "./lockfile.js" +export { getLockfileExpertToolDefinitions } from "./lockfile.js" export { calculateContextWindowUsage, getContextWindow } from "./model.js" export { getCurrentRuntimeVersion, diff --git a/apps/runtime/src/helpers/lockfile.test.ts b/apps/runtime/src/helpers/lockfile.test.ts index a4b7f113..0e713236 100644 --- a/apps/runtime/src/helpers/lockfile.test.ts +++ b/apps/runtime/src/helpers/lockfile.test.ts @@ -1,8 +1,6 @@ -import fs from "node:fs" -import path from "node:path" import type { LockfileExpert, LockfileToolDefinition } from "@perstack/core" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" -import { findLockfile, getLockfileExpertToolDefinitions, loadLockfile } from "./lockfile.js" +import { describe, expect, it } from "vitest" +import { getLockfileExpertToolDefinitions } from "./lockfile.js" const createLockfileExpert = ( toolDefinitions: LockfileToolDefinition[], @@ -19,64 +17,49 @@ const createLockfileExpert = ( ...overrides, }) -describe("lockfile", () => { - describe("getLockfileExpertToolDefinitions", () => { - it("groups tool definitions by skill name", () => { - const lockfileExpert = createLockfileExpert([ - { - skillName: "@perstack/base", - name: "readFile", - description: "Read a file", - inputSchema: { type: "object", properties: { path: { type: "string" } } }, - }, - { - skillName: "@perstack/base", - name: "writeFile", - description: "Write a file", - inputSchema: { type: "object", properties: { path: { type: "string" } } }, - }, - { - skillName: "other-skill", - name: "otherTool", - description: "Other tool", - inputSchema: { type: "object" }, - }, - ]) - - const result = getLockfileExpertToolDefinitions(lockfileExpert) - - expect(result["@perstack/base"]).toHaveLength(2) - expect(result["other-skill"]).toHaveLength(1) - expect(result["@perstack/base"][0].name).toBe("readFile") - expect(result["@perstack/base"][1].name).toBe("writeFile") - expect(result["other-skill"][0].name).toBe("otherTool") - }) +describe("getLockfileExpertToolDefinitions", () => { + it("groups tool definitions by skill name", () => { + const lockfileExpert = createLockfileExpert([ + { + skillName: "@perstack/base", + name: "readFile", + description: "Read a file", + inputSchema: { type: "object", properties: { path: { type: "string" } } }, + }, + { + skillName: "@perstack/base", + name: "writeFile", + description: "Write a file", + inputSchema: { type: "object", properties: { path: { type: "string" } } }, + }, + { + skillName: "other-skill", + name: "otherTool", + description: "Other tool", + inputSchema: { type: "object" }, + }, + ]) + + const result = getLockfileExpertToolDefinitions(lockfileExpert) + + expect(result["@perstack/base"]).toHaveLength(2) + expect(result["other-skill"]).toHaveLength(1) + expect(result["@perstack/base"][0].name).toBe("readFile") + expect(result["@perstack/base"][1].name).toBe("writeFile") + expect(result["other-skill"][0].name).toBe("otherTool") + }) - it("returns empty object for expert with no tool definitions", () => { - const lockfileExpert = createLockfileExpert([], { key: "empty-expert", name: "Empty Expert" }) + it("returns empty object for expert with no tool definitions", () => { + const lockfileExpert = createLockfileExpert([], { key: "empty-expert", name: "Empty Expert" }) - const result = getLockfileExpertToolDefinitions(lockfileExpert) + const result = getLockfileExpertToolDefinitions(lockfileExpert) - expect(Object.keys(result)).toHaveLength(0) - }) - - it("preserves tool definition properties", () => { - const lockfileExpert = createLockfileExpert([ - { - skillName: "test-skill", - name: "testTool", - description: "A test tool", - inputSchema: { - type: "object", - properties: { param: { type: "string" } }, - required: ["param"], - }, - }, - ]) - - const result = getLockfileExpertToolDefinitions(lockfileExpert) + expect(Object.keys(result)).toHaveLength(0) + }) - expect(result["test-skill"][0]).toEqual({ + it("preserves tool definition properties", () => { + const lockfileExpert = createLockfileExpert([ + { skillName: "test-skill", name: "testTool", description: "A test tool", @@ -85,163 +68,20 @@ describe("lockfile", () => { properties: { param: { type: "string" } }, required: ["param"], }, - }) - }) - }) - - describe("loadLockfile", () => { - const testDir = path.join(process.cwd(), ".test-lockfile-temp") - const testLockfilePath = path.join(testDir, "perstack.lock") - - beforeEach(() => { - if (!fs.existsSync(testDir)) { - fs.mkdirSync(testDir, { recursive: true }) - } - }) - - afterEach(() => { - vi.restoreAllMocks() - if (fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }) - } - }) - - it("returns parsed lockfile for valid TOML", () => { - const validToml = ` -version = "1" -generatedAt = 1704067200000 -configPath = "perstack.toml" - -[experts.test-expert] -key = "test-expert" -name = "Test Expert" -version = "1.0.0" -instruction = "Test" -delegates = [] -tags = [] -toolDefinitions = [] - -[experts.test-expert.skills] -` - fs.writeFileSync(testLockfilePath, validToml) - - const result = loadLockfile(testLockfilePath) - - expect(result).not.toBeNull() - expect(result?.version).toBe("1") - expect(result?.configPath).toBe("perstack.toml") - expect(Object.keys(result?.experts ?? {})).toHaveLength(1) - }) - - it("returns null for invalid TOML", () => { - fs.writeFileSync(testLockfilePath, "invalid { toml content") - - const result = loadLockfile(testLockfilePath) - - expect(result).toBeNull() - }) - - it("returns null for non-existent file", () => { - const result = loadLockfile("/non/existent/path/perstack.lock") - - expect(result).toBeNull() - }) - - it("returns null for TOML that doesn't match schema", () => { - fs.writeFileSync(testLockfilePath, "[invalid]\nkey = 'value'") - - const result = loadLockfile(testLockfilePath) - - expect(result).toBeNull() - }) - }) - - describe("findLockfile", () => { - const originalCwd = process.cwd() - const testDir = path.join(originalCwd, ".test-find-lockfile-temp") - const nestedDir = path.join(testDir, "nested", "deep") - - beforeEach(() => { - if (!fs.existsSync(nestedDir)) { - fs.mkdirSync(nestedDir, { recursive: true }) - } - }) - - afterEach(() => { - vi.restoreAllMocks() - if (fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }) - } - }) - - it("returns lockfile path based on config path when provided", () => { - const configPath = path.join(testDir, "perstack.toml") - - const result = findLockfile(configPath) - - expect(result).toBe(path.join(testDir, "perstack.lock")) - }) - - it("returns lockfile path from config path even if file does not exist", () => { - // findLockfile with configPath always returns the path, doesn't check existence - const configPath = path.join("/some/nonexistent", "perstack.toml") - - const result = findLockfile(configPath) - - expect(result).toBe(path.join("/some/nonexistent", "perstack.lock")) - }) - - it("returns null for remote config URLs (https://)", () => { - const configPath = "https://raw.githubusercontent.com/org/repo/main/perstack.toml" - - const result = findLockfile(configPath) - - expect(result).toBeNull() - }) - - it("returns null for remote config URLs (http://)", () => { - const configPath = "http://example.com/perstack.toml" - - const result = findLockfile(configPath) - - expect(result).toBeNull() - }) - - it("returns null for remote config URLs with uppercase scheme (HTTPS://)", () => { - const configPath = "HTTPS://example.com/perstack.toml" - - const result = findLockfile(configPath) - - expect(result).toBeNull() - }) - - it("returns null for remote config URLs with mixed case scheme (HtTpS://)", () => { - const configPath = "HtTpS://raw.githubusercontent.com/org/repo/main/perstack.toml" - - const result = findLockfile(configPath) - - expect(result).toBeNull() - }) - - it("finds lockfile recursively from nested directory", () => { - // Create lockfile in parent directory - const lockfilePath = path.join(testDir, "perstack.lock") - fs.writeFileSync(lockfilePath, "") - vi.spyOn(process, "cwd").mockReturnValue(nestedDir) - - const result = findLockfile() - - expect(result).toBe(lockfilePath) - }) - - it("finds lockfile in current directory", () => { - const lockfilePath = path.join(testDir, "perstack.lock") - fs.writeFileSync(lockfilePath, "") - vi.spyOn(process, "cwd").mockReturnValue(testDir) - - const result = findLockfile() - - expect(result).toBe(lockfilePath) + }, + ]) + + const result = getLockfileExpertToolDefinitions(lockfileExpert) + + expect(result["test-skill"][0]).toEqual({ + skillName: "test-skill", + name: "testTool", + description: "A test tool", + inputSchema: { + type: "object", + properties: { param: { type: "string" } }, + required: ["param"], + }, }) }) }) diff --git a/apps/runtime/src/helpers/lockfile.ts b/apps/runtime/src/helpers/lockfile.ts index 755701b4..a638f34b 100644 --- a/apps/runtime/src/helpers/lockfile.ts +++ b/apps/runtime/src/helpers/lockfile.ts @@ -1,52 +1,4 @@ -import { readFileSync } from "node:fs" -import path from "node:path" -import { - type Lockfile, - type LockfileExpert, - lockfileSchema, - parseWithFriendlyError, -} from "@perstack/core" -import TOML from "smol-toml" - -export function loadLockfile(lockfilePath: string): Lockfile | null { - try { - const content = readFileSync(lockfilePath, "utf-8") - const parsed = TOML.parse(content) - return parseWithFriendlyError(lockfileSchema, parsed, "perstack.lock") - } catch { - return null - } -} - -function isRemoteUrl(configPath: string): boolean { - const lower = configPath.toLowerCase() - return lower.startsWith("https://") || lower.startsWith("http://") -} - -export function findLockfile(configPath?: string): string | null { - if (configPath) { - // Remote config URLs don't support lockfiles - skip silently - if (isRemoteUrl(configPath)) { - return null - } - const configDir = path.dirname(path.resolve(process.cwd(), configPath)) - return path.join(configDir, "perstack.lock") - } - return findLockfileRecursively(process.cwd()) -} - -function findLockfileRecursively(cwd: string): string | null { - const lockfilePath = path.resolve(cwd, "perstack.lock") - try { - readFileSync(lockfilePath) - return lockfilePath - } catch { - if (cwd === path.parse(cwd).root) { - return null - } - return findLockfileRecursively(path.dirname(cwd)) - } -} +import type { LockfileExpert } from "@perstack/core" export function getLockfileExpertToolDefinitions( lockfileExpert: LockfileExpert, diff --git a/apps/runtime/src/index.ts b/apps/runtime/src/index.ts index 2d8ca7c4..d35c4861 100755 --- a/apps/runtime/src/index.ts +++ b/apps/runtime/src/index.ts @@ -1,6 +1,6 @@ import pkg from "../package.json" with { type: "json" } -export { findLockfile, getLockfileExpertToolDefinitions, loadLockfile } from "./helpers/index.js" +export { getLockfileExpertToolDefinitions } from "./helpers/index.js" export { type RunOptions, run } from "./run.js" export { type CollectedToolDefinition, diff --git a/packages/tui/src/lockfile.test.ts b/packages/tui/src/lockfile.test.ts new file mode 100644 index 00000000..2e9a8d82 --- /dev/null +++ b/packages/tui/src/lockfile.test.ts @@ -0,0 +1,162 @@ +import fs from "node:fs" +import path from "node:path" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { findLockfile, loadLockfile } from "./lockfile.js" + +describe("lockfile", () => { + describe("loadLockfile", () => { + const testDir = path.join(process.cwd(), ".test-lockfile-temp") + const testLockfilePath = path.join(testDir, "perstack.lock") + + beforeEach(() => { + if (!fs.existsSync(testDir)) { + fs.mkdirSync(testDir, { recursive: true }) + } + }) + + afterEach(() => { + vi.restoreAllMocks() + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }) + } + }) + + it("returns parsed lockfile for valid TOML", () => { + const validToml = ` +version = "1" +generatedAt = 1704067200000 +configPath = "perstack.toml" + +[experts.test-expert] +key = "test-expert" +name = "Test Expert" +version = "1.0.0" +instruction = "Test" +delegates = [] +tags = [] +toolDefinitions = [] + +[experts.test-expert.skills] +` + fs.writeFileSync(testLockfilePath, validToml) + + const result = loadLockfile(testLockfilePath) + + expect(result).not.toBeNull() + expect(result?.version).toBe("1") + expect(result?.configPath).toBe("perstack.toml") + expect(Object.keys(result?.experts ?? {})).toHaveLength(1) + }) + + it("returns null for invalid TOML", () => { + fs.writeFileSync(testLockfilePath, "invalid { toml content") + + const result = loadLockfile(testLockfilePath) + + expect(result).toBeNull() + }) + + it("returns null for non-existent file", () => { + const result = loadLockfile("/non/existent/path/perstack.lock") + + expect(result).toBeNull() + }) + + it("returns null for TOML that doesn't match schema", () => { + fs.writeFileSync(testLockfilePath, "[invalid]\nkey = 'value'") + + const result = loadLockfile(testLockfilePath) + + expect(result).toBeNull() + }) + }) + + describe("findLockfile", () => { + const originalCwd = process.cwd() + const testDir = path.join(originalCwd, ".test-find-lockfile-temp") + const nestedDir = path.join(testDir, "nested", "deep") + + beforeEach(() => { + if (!fs.existsSync(nestedDir)) { + fs.mkdirSync(nestedDir, { recursive: true }) + } + }) + + afterEach(() => { + vi.restoreAllMocks() + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }) + } + }) + + it("returns lockfile path based on config path when provided", () => { + const configPath = path.join(testDir, "perstack.toml") + + const result = findLockfile(configPath) + + expect(result).toBe(path.join(testDir, "perstack.lock")) + }) + + it("returns lockfile path from config path even if file does not exist", () => { + // findLockfile with configPath always returns the path, doesn't check existence + const configPath = path.join("/some/nonexistent", "perstack.toml") + + const result = findLockfile(configPath) + + expect(result).toBe(path.join("/some/nonexistent", "perstack.lock")) + }) + + it("returns null for remote config URLs (https://)", () => { + const configPath = "https://raw.githubusercontent.com/org/repo/main/perstack.toml" + + const result = findLockfile(configPath) + + expect(result).toBeNull() + }) + + it("returns null for remote config URLs (http://)", () => { + const configPath = "http://example.com/perstack.toml" + + const result = findLockfile(configPath) + + expect(result).toBeNull() + }) + + it("returns null for remote config URLs with uppercase scheme (HTTPS://)", () => { + const configPath = "HTTPS://example.com/perstack.toml" + + const result = findLockfile(configPath) + + expect(result).toBeNull() + }) + + it("returns null for remote config URLs with mixed case scheme (HtTpS://)", () => { + const configPath = "HtTpS://raw.githubusercontent.com/org/repo/main/perstack.toml" + + const result = findLockfile(configPath) + + expect(result).toBeNull() + }) + + it("finds lockfile recursively from nested directory", () => { + // Create lockfile in parent directory + const lockfilePath = path.join(testDir, "perstack.lock") + fs.writeFileSync(lockfilePath, "") + vi.spyOn(process, "cwd").mockReturnValue(nestedDir) + + const result = findLockfile() + + expect(result).toBe(lockfilePath) + }) + + it("finds lockfile in current directory", () => { + const lockfilePath = path.join(testDir, "perstack.lock") + fs.writeFileSync(lockfilePath, "") + vi.spyOn(process, "cwd").mockReturnValue(testDir) + + const result = findLockfile() + + expect(result).toBe(lockfilePath) + }) + }) +}) diff --git a/packages/tui/src/lockfile.ts b/packages/tui/src/lockfile.ts new file mode 100644 index 00000000..3960a956 --- /dev/null +++ b/packages/tui/src/lockfile.ts @@ -0,0 +1,44 @@ +import { readFileSync } from "node:fs" +import path from "node:path" +import { type Lockfile, lockfileSchema, parseWithFriendlyError } from "@perstack/core" +import TOML from "smol-toml" + +export function loadLockfile(lockfilePath: string): Lockfile | null { + try { + const content = readFileSync(lockfilePath, "utf-8") + const parsed = TOML.parse(content) + return parseWithFriendlyError(lockfileSchema, parsed, "perstack.lock") + } catch { + return null + } +} + +function isRemoteUrl(configPath: string): boolean { + const lower = configPath.toLowerCase() + return lower.startsWith("https://") || lower.startsWith("http://") +} + +export function findLockfile(configPath?: string): string | null { + if (configPath) { + // Remote config URLs don't support lockfiles - skip silently + if (isRemoteUrl(configPath)) { + return null + } + const configDir = path.dirname(path.resolve(process.cwd(), configPath)) + return path.join(configDir, "perstack.lock") + } + return findLockfileRecursively(process.cwd()) +} + +function findLockfileRecursively(cwd: string): string | null { + const lockfilePath = path.resolve(cwd, "perstack.lock") + try { + readFileSync(lockfilePath) + return lockfilePath + } catch { + if (cwd === path.parse(cwd).root) { + return null + } + return findLockfileRecursively(path.dirname(cwd)) + } +} diff --git a/packages/tui/src/run-handler.ts b/packages/tui/src/run-handler.ts index 2ee58375..8a1f407a 100644 --- a/packages/tui/src/run-handler.ts +++ b/packages/tui/src/run-handler.ts @@ -14,12 +14,13 @@ import { retrieveJob, storeJob, } from "@perstack/filesystem-storage" -import { findLockfile, loadLockfile, run as perstackRun } from "@perstack/runtime" +import { run as perstackRun } from "@perstack/runtime" import { resolveRunContext } from "./lib/context.js" import { parseInteractiveToolCallResult, parseInteractiveToolCallResultJson, } from "./lib/interactive.js" +import { findLockfile, loadLockfile } from "./lockfile.js" const defaultEventListener = (event: RunEvent | RuntimeEvent) => console.log(JSON.stringify(event)) diff --git a/packages/tui/src/start-handler.ts b/packages/tui/src/start-handler.ts index b4c756b7..9ce50091 100644 --- a/packages/tui/src/start-handler.ts +++ b/packages/tui/src/start-handler.ts @@ -14,7 +14,7 @@ import { retrieveJob, storeJob, } from "@perstack/filesystem-storage" -import { findLockfile, loadLockfile, run as perstackRun, runtimeVersion } from "@perstack/runtime" +import { run as perstackRun, runtimeVersion } from "@perstack/runtime" import { type CheckpointHistoryItem, type JobHistoryItem, @@ -31,6 +31,7 @@ import { type getEventContents, getRecentExperts, } from "./lib/run-manager.js" +import { findLockfile, loadLockfile } from "./lockfile.js" const CONTINUE_TIMEOUT_MS = 60_000