diff --git a/docs/convention.md b/docs/convention.md index 694c8d3..57a3542 100644 --- a/docs/convention.md +++ b/docs/convention.md @@ -34,17 +34,11 @@ Hindsight client and server remain responsible for bank routing, credentials, and runtime health. Existing configurations without `memory` retain their current behavior; `dailyLogs` and `wiki` remain the legacy llm-wiki signal. -The strategy describes who owns _repository-authored_ memory. It says nothing -about an agent runtime that writes into the checkout on its own schedule. -Hindsight and a runtime's local memory are complementary, not alternatives: -Hindsight retains cross-session experience, while a runtime such as OpenClaw -consolidates its own local state. Declaring `hindsight` therefore does not mean -`memory/` must be absent, and a workspace must not add such a path to -`forbidden` — the kit rejects that rule, because it converts scheduled runtime -output into a gate failure. Ignore the path in `.gitignore` instead. - -The kit currently treats `memory/` and `DREAMS.md`, OpenClaw's dream diary, as -runtime-owned, along with `.openclaw-repair/` and anything nested under them. +Consumers decide which paths their runtime owns and which paths must be absent. +The memory strategy alone does not establish runtime ownership. Configure +`forbidden` for paths that this workspace must reject. For a workspace whose +runtime writes local memory, allow those paths and ignore generated output in +`.gitignore` when appropriate. ### LLM wiki @@ -138,6 +132,11 @@ then run: pnpm exec workspace-kit skills sync ``` +- Before changing files, sync validates declared destinations and stops at the + first conflict. Replacing an + existing remote copy requires matching source records in both ownership locks. + Unmanaged directories, symlinks, and mismatched provenance stop sync for + explicit owner handling. - Sync links every locally authored skill into `.agents/skills`, ensures the Claude discovery link, then delegates each remote entry to a pinned `skills` CLI with telemetry disabled, project scope, and copy mode. @@ -371,6 +370,10 @@ repository-history security scans remain independently operated surfaces. - Existing files remain unchanged. A re-run accepts a compatible `package.json`; an existing repository follows the adoption steps above, and init stops before writing when its package contract is incompatible. +- A re-run follows the existing memory configuration, including legacy + `dailyLogs`/`wiki` sections. Conflicting explicit memory options fail before + scaffolding, as does an unreadable existing config. Changing memory strategy + is an owner-managed migration. - `work`: AGENTS.md + CLAUDE.md symlink + package.json + docs/README.md + workspace.json. - `personal`: work + README, `.env.example`, project-registry stub and diff --git a/src/config.ts b/src/config.ts index fd617db..23ae145 100644 --- a/src/config.ts +++ b/src/config.ts @@ -76,21 +76,6 @@ function stringList(value: unknown, field: string): string[] { return value as string[]; } -/** - * Paths an agent runtime writes into a consumer workspace. A consumer that - * forbids one turns scheduled runtime output into a gate failure, so the kit - * rejects the rule rather than letting the two systems disagree. - * - * OpenClaw's memory-core dreaming job owns `memory/` and writes its dream diary - * to `DREAMS.md` in the workspace root. `.openclaw-repair/` holds root-memory - * migration state. - */ -const RUNTIME_OWNED_PATHS = ["memory", "DREAMS.md", ".openclaw-repair"]; - -function runtimeOwnedPath(path: string): string | undefined { - return RUNTIME_OWNED_PATHS.find((owned) => path === owned || path.startsWith(`${owned}/`)); -} - function workspacePathList(value: unknown, field: string): string[] { return stringList(value, field).map((path, index) => normalizeWorkspacePath(path, `${field}[${index}]`), @@ -216,20 +201,7 @@ export function parseWorkspaceConfig(value: unknown): WorkspaceConfig { out.minVersion = minVersion; } if ("required" in value) out.required = workspacePathList(value.required, "required"); - if ("forbidden" in value) { - const forbidden = workspacePathList(value.forbidden, "forbidden"); - for (const path of forbidden) { - const owned = runtimeOwnedPath(path); - if (owned) { - fail( - `forbidden must not list ${path}: an agent runtime owns ${owned} and ` + - "writes to it on a schedule. Ignore it in .gitignore instead of " + - "failing the workspace gate on runtime output.", - ); - } - } - out.forbidden = forbidden; - } + if ("forbidden" in value) out.forbidden = workspacePathList(value.forbidden, "forbidden"); if ("links" in value) { if (!Array.isArray(value.links)) fail("links must be an array"); diff --git a/src/init.ts b/src/init.ts index e3e1a60..a289714 100644 --- a/src/init.ts +++ b/src/init.ts @@ -168,11 +168,29 @@ export function initWorkspace( : requestedMemory; const root = realpathSync(dir); assertCompatiblePackage(root, profile); - const memory = - validatedRequestedMemory ?? - (profile === "personal" || profile === "runtime" + const existingConfig = workspaceLstat(root, "workspace.json") + ? parseWorkspaceConfig(JSON.parse(readWorkspaceText(root, "workspace.json"))) + : undefined; + const existingMemory = + existingConfig?.memory ?? + (existingConfig?.dailyLogs || existingConfig?.wiki ? ({ strategy: "llm-wiki" } as const) : undefined); + if ( + existingConfig && + validatedRequestedMemory && + JSON.stringify(validatedRequestedMemory) !== JSON.stringify(existingMemory) + ) { + throw new Error( + "requested memory configuration conflicts with workspace.json; update the existing workspace explicitly", + ); + } + const memory = existingConfig + ? existingMemory + : (validatedRequestedMemory ?? + (profile === "personal" || profile === "runtime" + ? ({ strategy: "llm-wiki" } as const) + : undefined)); const seedWikiCatalog = memory?.strategy === "llm-wiki" && !workspaceLstat(root, "workspace.json") && diff --git a/src/skills.ts b/src/skills.ts index fe307dd..81d7e60 100644 --- a/src/skills.ts +++ b/src/skills.ts @@ -415,6 +415,22 @@ export function syncWorkspaceSkills( } } + for (const { name } of skills.remote) { + const runtimePath = `.agents/skills/${name}`; + const stat = workspaceLstat(repoRoot, runtimePath, "remote runtime skill"); + if (!stat) continue; + if (stat.isSymbolicLink() || !stat.isDirectory()) { + return [`${runtimePath} is not a managed copied directory`]; + } + const source = managed.get(name); + const dependencyEntry = dependencyLock?.skills[name]; + if (!source || !isRecord(dependencyEntry) || dependencyEntry.source !== source) { + return [ + `${runtimePath} exists without matching workspace-kit and dependency ownership; preserve or move it before syncing`, + ]; + } + } + ensureWorkspaceDirectory(repoRoot, ".agents/skills"); if (!claude) createWorkspaceLink(repoRoot, ".claude/skills", "../.agents/skills"); } catch (error) { diff --git a/src/version.ts b/src/version.ts index fb75307..786b624 100644 --- a/src/version.ts +++ b/src/version.ts @@ -3,6 +3,8 @@ import { existsSync, readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; +import { gitEnvironmentForRepository } from "./lib/gitProcess.ts"; + declare const __WORKSPACE_KIT_VERSION__: string | undefined; const STABLE_VERSION = /^\d+\.\d+\.\d+$/; @@ -40,6 +42,7 @@ function reachableTagVersions(packageRoot: string): string[] { const shallow = spawnSync("git", ["rev-parse", "--is-shallow-repository"], { cwd: packageRoot, encoding: "utf8", + env: gitEnvironmentForRepository(), }); if (shallow.status !== 0) { throw new Error("could not inspect Git history while determining workspace-kit version"); @@ -52,6 +55,7 @@ function reachableTagVersions(packageRoot: string): string[] { const result = spawnSync("git", ["tag", "--merged", "HEAD", "--list", "v*"], { cwd: packageRoot, encoding: "utf8", + env: gitEnvironmentForRepository(), }); if (result.status !== 0) { throw new Error("could not read Git tags while determining workspace-kit version"); diff --git a/test/cli-commands.test.ts b/test/cli-commands.test.ts index 5e1cc4f..6854ba9 100644 --- a/test/cli-commands.test.ts +++ b/test/cli-commands.test.ts @@ -334,8 +334,12 @@ test("init refuses to write through pre-existing dangling symlinks", () => { const elsewhere = mkdtempSync(join(tmpdir(), "elsewhere-")); const plantedTarget = join(elsewhere, "planted.json"); symlinkSync(plantedTarget, join(victim, "workspace.json")); - const result = initWorkspace(victim, "work"); - assert.ok(result.skipped.includes("workspace.json")); + assert.throws( + () => initWorkspace(victim, "work"), + /workspace.json: symbolic-link file is not allowed/, + ); + assert.ok(lstatSync(join(victim, "workspace.json")).isSymbolicLink()); + assert.equal(existsSync(join(victim, "AGENTS.md")), false); assert.ok(!existsSync(plantedTarget), "must not create files at the symlink target"); }); diff --git a/test/config-version-handoff.test.ts b/test/config-version-handoff.test.ts index bd9c50f..50e3073 100644 --- a/test/config-version-handoff.test.ts +++ b/test/config-version-handoff.test.ts @@ -365,6 +365,22 @@ test("source version requires full tag history and prefers a newer stamped packa ]); git(checkout, ["-c", "tag.gpgSign=false", "tag", "v0.3.0"]); assert.equal(resolveKitVersion(checkout), "0.3.0"); + const foreign = join(cloneParent, "foreign"); + git(cloneParent, ["clone", "-q", checkout, foreign]); + git(foreign, ["-c", "tag.gpgSign=false", "tag", "v9.9.9"]); + const resolver = new URL("../src/version.ts", import.meta.url).href; + const isolated = spawnSync( + process.execPath, + [ + "--input-type=module", + "-e", + `import { resolveKitVersion } from ${JSON.stringify(resolver)}; console.log(resolveKitVersion(process.argv[1]));`, + checkout, + ], + { encoding: "utf8", env: { ...process.env, GIT_DIR: join(foreign, ".git") } }, + ); + assert.equal(isolated.status, 0, isolated.stderr); + assert.equal(isolated.stdout.trim(), "0.3.0"); writeFileSync(join(checkout, "marker.txt"), "after release\n"); git(checkout, ["add", "marker.txt"]); @@ -400,18 +416,16 @@ test("source version requires full tag history and prefers a newer stamped packa } }); -test("forbidden rejects runtime-owned paths", () => { - assert.throws( - () => parseWorkspaceConfig({ minVersion: "0.13.4", forbidden: ["memory"] }), - /an agent runtime owns/, - ); -}); - -test("forbidden rejects paths nested under a runtime-owned root", () => { - for (const path of ["memory/dreaming", "DREAMS.md", ".openclaw-repair"]) { - assert.throws( - () => parseWorkspaceConfig({ minVersion: "0.13.4", forbidden: [path] }), - /an agent runtime owns/, +test("forbidden paths remain consumer policy regardless of memory integration", () => { + const forbidden = ["memory", "memory/dreaming", "DREAMS.md", ".openclaw-repair"]; + assert.deepEqual(parseWorkspaceConfig({ forbidden }).forbidden, forbidden); + for (const integration of ["coding-agent", "openclaw"]) { + assert.deepEqual( + parseWorkspaceConfig({ + forbidden, + memory: { strategy: "hindsight", integration, namespace: "fixture-owner/workspace" }, + }).forbidden, + forbidden, ); } }); diff --git a/test/init.test.ts b/test/init.test.ts index 4ca561b..5411169 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -11,6 +11,7 @@ import { mkdtempSync, readFileSync, readlinkSync, + rmSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -219,3 +220,67 @@ test("init validates explicit memory selections before writing", () => { function scratchDirectory(prefix: string): string { return mkdtempSync(join(tmpdir(), prefix)); } + +for (const profile of ["personal", "runtime"] as const) { + test(`reinitializing ${profile} preserves existing Hindsight and rejects migration options`, () => { + const dir = scratchDirectory("reinit-memory-"); + const memory = { + strategy: "hindsight", + integration: "coding-agent", + namespace: "fixture-owner/workspace", + } as const; + initWorkspace(dir, profile, memory); + const before = readFileSync(join(dir, "workspace.json"), "utf8"); + assert.equal(initWorkspace(dir, profile).created.length, 0); + assert.equal(initWorkspace(dir, profile, memory).created.length, 0); + assert.equal(existsSync(join(dir, "memory/wiki")), false); + rmSync(join(dir, "README.md")); + for (const requested of [ + { strategy: "llm-wiki" } as const, + { ...memory, namespace: "fixture-owner/other" }, + { ...memory, integration: "openclaw" } as const, + ]) { + assert.throws(() => initWorkspace(dir, profile, requested), /conflicts with workspace.json/); + assert.equal(existsSync(join(dir, "README.md")), false); + assert.equal(existsSync(join(dir, "memory/wiki")), false); + assert.equal(readFileSync(join(dir, "workspace.json"), "utf8"), before); + } + }); +} + +test("init preserves legacy or disabled memory and refuses malformed existing configuration", () => { + const legacy = scratchDirectory("init-legacy-memory-"); + initWorkspace(legacy, "personal"); + const config = JSON.parse(readFileSync(join(legacy, "workspace.json"), "utf8")); + delete config.memory; + writeFileSync(join(legacy, "workspace.json"), JSON.stringify(config)); + assert.equal(initWorkspace(legacy, "personal").created.length, 0); + const saved = JSON.parse(readFileSync(join(legacy, "workspace.json"), "utf8")); + assert.deepEqual(saved, config); + assert.deepEqual(saved.dailyLogs, { root: "memory", contexts: "memory/contexts" }); + assert.deepEqual(saved.wiki, { root: "memory/wiki" }); + assert.equal(initWorkspace(legacy, "personal", { strategy: "llm-wiki" }).created.length, 0); + assert.throws( + () => + initWorkspace(legacy, "personal", { + strategy: "hindsight", + integration: "coding-agent", + namespace: "fixture-owner/workspace", + }), + /conflicts with workspace.json/, + ); + + const disabled = scratchDirectory("init-disabled-memory-"); + writeFileSync(join(disabled, "workspace.json"), "{}"); + initWorkspace(disabled, "personal"); + assert.equal(existsSync(join(disabled, "memory/wiki")), false); + assert.throws(() => initWorkspace(disabled, "personal", { strategy: "llm-wiki" }), /conflicts/); + + const invalid = scratchDirectory("init-invalid-config-"); + writeFileSync(join(invalid, "workspace.json"), '{"memory":{"strategy":"invalid"}}'); + assert.throws(() => initWorkspace(invalid, "personal"), /memory.strategy/); + assert.equal(existsSync(join(invalid, "AGENTS.md")), false); + writeFileSync(join(invalid, "workspace.json"), "{"); + assert.throws(() => initWorkspace(invalid, "personal"), SyntaxError); + assert.equal(existsSync(join(invalid, "AGENTS.md")), false); +}); diff --git a/test/skills.test.ts b/test/skills.test.ts index 597159f..9e17789 100644 --- a/test/skills.test.ts +++ b/test/skills.test.ts @@ -547,3 +547,95 @@ test("skills check and doctor expose the offline contract", () => { errors: [], }); }); + +test("remote installation preflights all destinations before any mutation", () => { + for (const ownership of ["none", "dependency-only", "manager-only", "mismatch"]) { + const root = scratch(); + const remote = { name: "remote-skill", source: "fixture/remote" }; + const retired = { name: "retired-skill", source: "fixture/retired" }; + writeManifest(root, [remote]); + writeDiscovery(root); + writeSkill(root, ".agents/skills", remote.name); + writeSkill(root, ".agents/skills", retired.name); + const note = join(root, ".agents/skills", remote.name, "owner-note.txt"); + writeFileSync(note, "keep owner content"); + writeLock(root, [ + retired, + ...(ownership === "dependency-only" + ? [remote] + : ownership === "mismatch" + ? [{ ...remote, source: "fixture/replacement" }] + : []), + ]); + writeManagedLock(root, [ + retired, + ...(ownership === "manager-only" || ownership === "mismatch" ? [remote] : []), + ]); + const lockBefore = readFileSync(join(root, "skills/workspace-kit-lock.json")); + let calls = 0; + const errors = syncWorkspaceSkills(root, config, () => { + calls++; + return { status: 0 }; + }); + assert.match(errors[0] ?? "", /without matching.*ownership/); + assert.equal(calls, 0); + assert.equal(readFileSync(note, "utf8"), "keep owner content"); + assert.ok(workspaceLstatForTest(root, ".agents/skills/retired-skill")); + assert.deepEqual(readFileSync(join(root, "skills/workspace-kit-lock.json")), lockBefore); + } +}); + +test("remote installation rejects occupied files and symlinks even with matching locks", () => { + for (const shape of ["file", "symlink"]) { + const root = scratch(); + const remote = { name: "remote-skill", source: "fixture/remote" }; + writeManifest(root, [remote]); + writeDiscovery(root); + writeLock(root, [remote]); + writeManagedLock(root, [remote]); + const path = join(root, ".agents/skills/remote-skill"); + if (shape === "file") writeFileSync(path, "keep"); + else symlinkSync("../../skills/remote-skill", path); + let called = false; + assert.match( + syncWorkspaceSkills(root, config, () => { + called = true; + return { status: 0 }; + })[0] ?? "", + /not a managed copied directory/, + ); + assert.equal(called, false); + if (shape === "file") assert.equal(readFileSync(path, "utf8"), "keep"); + else assert.equal(readlinkSync(path), "../../skills/remote-skill"); + } +}); + +test("remote installation updates proven managed copies including declared source changes", () => { + for (const source of ["fixture/original", "fixture/replacement"]) { + const root = scratch(); + const original = { name: "remote-skill", source: "fixture/original" }; + const requested = { ...original, source }; + writeManifest(root, [requested]); + writeDiscovery(root); + writeSkill(root, ".agents/skills", original.name); + writeLock(root, [original]); + writeManagedLock(root, [original]); + let calls = 0; + assert.deepEqual( + syncWorkspaceSkills(root, config, (_command, args) => { + calls++; + assert.equal(args[3], source); + writeLock(root, [requested]); + return { status: 0 }; + }), + [], + ); + assert.equal(calls, 1); + assert.equal( + JSON.parse(readFileSync(join(root, "skills/workspace-kit-lock.json"), "utf8")).skills[ + original.name + ], + source, + ); + } +});