From 5c229cb5694950350f8f76318c76d2e2458cad0b Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 11:39:11 -0500 Subject: [PATCH 1/9] =?UTF-8?q?RT:=20home-repo=20reroot=20task=201=20?= =?UTF-8?q?=E2=80=94=20machineKey()=20+=20settings-store=20path=20construc?= =?UTF-8?q?tors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the three settings-store paths to the new user-repo layout and adds machineKey() (override file, else slugified hostname) so the machine store nests per-machine under user/local//. lib/rt-paths.ts is the authority; packages/rt-client/src/settings/paths.ts mirrors it verbatim. Co-Authored-By: Claude Fable 5 --- lib/__tests__/rt-paths.test.ts | 92 +++++++++++++++- lib/__tests__/settings-paths-parity.test.ts | 30 ++++- lib/command-tree-def.ts | 2 +- lib/daemon/__tests__/doppler-sync.test.ts | 2 +- .../__tests__/worktree-reconciler.test.ts | 4 +- lib/rt-paths.ts | 70 ++++++++---- .../src/settings/__tests__/identity.test.ts | 8 +- .../src/settings/__tests__/paths.test.ts | 104 ++++++++++++++++++ .../src/settings/__tests__/stores.test.ts | 25 +++-- .../src/settings/__tests__/write.test.ts | 14 ++- packages/rt-client/src/settings/paths.ts | 48 ++++++-- packages/rt-client/src/settings/stores.ts | 2 +- 12 files changed, 349 insertions(+), 52 deletions(-) create mode 100644 packages/rt-client/src/settings/__tests__/paths.test.ts diff --git a/lib/__tests__/rt-paths.test.ts b/lib/__tests__/rt-paths.test.ts index cec22c65..0243c471 100644 --- a/lib/__tests__/rt-paths.test.ts +++ b/lib/__tests__/rt-paths.test.ts @@ -12,18 +12,20 @@ * here instead of a scavenger hunt. */ -import { describe, test, expect, afterEach } from "bun:test"; +import { describe, test, expect, afterEach, mock } from "bun:test"; import { existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync, } from "fs"; +import * as osReal from "os"; import { tmpdir } from "os"; -import { join } from "path"; +import { dirname, join } from "path"; import { rtDir, reposDir, repoDataDir, logsDir, migrateLegacyRtDir, legacyDirsPresent, TRAY_APP_NAME, DEV_TRAY_APP_NAME, TRAY_APP_BUNDLE, DEV_TRAY_APP_BUNDLE, trayAppPath, devTrayAppPath, legacyTrayAppPaths, installedTrayAppPath, machineSettingsPath, + machineKey, userSettingsPath, teamSettingsPath, } from "../rt-paths.ts"; describe("rt-paths", () => { @@ -70,6 +72,86 @@ describe("rt-paths", () => { expect(repoDataDir("x")).not.toBe(join(rtDir(), "x")); }); + // ── Settings store paths (home-repo reroot) ───────────────────────────────── + + describe("settings store path shapes", () => { + test("userSettingsPath nests under user/settings.user.jsonc", () => { + process.env.HOME = "/tmp/fake-home-store-1"; + expect(userSettingsPath()).toBe("/tmp/fake-home-store-1/.mattstack/user/settings.user.jsonc"); + }); + + test("machineSettingsPath nests under user/local//settings.local.jsonc", () => { + const home = mkdtempSync(join(tmpdir(), "rt-paths-machine-store-")); + process.env.HOME = home; + expect(machineSettingsPath()).toBe(join(home, ".mattstack", "user", "local", machineKey(), "settings.local.jsonc")); + rmSync(home, { recursive: true, force: true }); + }); + + test("teamSettingsPath nests under teams//mattstack/settings.team.jsonc", () => { + process.env.HOME = "/tmp/fake-home-store-2"; + expect(teamSettingsPath("acme")).toBe("/tmp/fake-home-store-2/.mattstack/teams/acme/mattstack/settings.team.jsonc"); + }); + }); + + describe("machineKey", () => { + const makeKeyHome = () => mkdtempSync(join(tmpdir(), "rt-paths-machine-key-")); + + afterEach(() => { + mock.module("os", () => osReal); + }); + + test("an override file wins, trimmed", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), " my-custom-key \n"); + expect(machineKey()).toBe("my-custom-key"); + rmSync(home, { recursive: true, force: true }); + }); + + test("an override file that is empty after trim falls through to the hostname slug", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), " \n"); + mock.module("os", () => ({ ...osReal, hostname: () => "Real-Host" })); + expect(machineKey()).toBe("real-host"); + rmSync(home, { recursive: true, force: true }); + }); + + test("no override file at all: falls through to the hostname slug", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "Some-Host" })); + expect(machineKey()).toBe("some-host"); + rmSync(home, { recursive: true, force: true }); + }); + + test("hostname slug: lowercased, trailing .local stripped", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "Matts-MacBook-Pro.local" })); + expect(machineKey()).toBe("matts-macbook-pro"); + rmSync(home, { recursive: true, force: true }); + }); + + test("hostname slug: illegal characters collapse to single dashes, edges trimmed", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => " weird_host!!name@@ " })); + expect(machineKey()).toBe("weird-host-name"); + rmSync(home, { recursive: true, force: true }); + }); + + test("hostname slug: an all-illegal hostname slugs to empty and falls back to \"default\"", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "!!!" })); + expect(machineKey()).toBe("default"); + rmSync(home, { recursive: true, force: true }); + }); + }); + // ── migrateLegacyRtDir ─────────────────────────────────────────────────────── const makeHome = () => mkdtempSync(join(tmpdir(), "rt-paths-migrate-")); @@ -219,7 +301,7 @@ describe("rt-paths", () => { test("the mattstack.appPath machine setting wins over both fixed locations", () => { const home = makeHome(); process.env.HOME = home; - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); writeFileSync(machineSettingsPath(), JSON.stringify({ "mattstack.appPath": "/custom/place/mattstack.app" })); const exists = (p: string) => p === "/custom/place/mattstack.app" || p === "/Applications/mattstack.app"; @@ -231,7 +313,7 @@ describe("rt-paths", () => { test("a machine setting naming a different bundle is never trusted for this lookup, even if it exists on disk", () => { const home = makeHome(); process.env.HOME = home; - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); // The setting names the PROD bundle; this call asks for the dev bundle. writeFileSync(machineSettingsPath(), JSON.stringify({ "mattstack.appPath": "/Applications/mattstack.app" })); @@ -244,7 +326,7 @@ describe("rt-paths", () => { test("a machine setting pointing at a bundle that no longer exists is not trusted — falls through to the fixed locations", () => { const home = makeHome(); process.env.HOME = home; - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); writeFileSync(machineSettingsPath(), JSON.stringify({ "mattstack.appPath": "/gone/mattstack.app" })); const exists = (p: string) => p === "/Applications/mattstack.app"; diff --git a/lib/__tests__/settings-paths-parity.test.ts b/lib/__tests__/settings-paths-parity.test.ts index ae141660..98d93ae2 100644 --- a/lib/__tests__/settings-paths-parity.test.ts +++ b/lib/__tests__/settings-paths-parity.test.ts @@ -8,6 +8,8 @@ */ import { describe, test, expect, afterEach } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; import { join } from "path"; import * as rtPaths from "../rt-paths.ts"; import * as clientPaths from "../../packages/rt-client/src/settings/paths.ts"; @@ -29,10 +31,34 @@ describe("settings paths parity (lib/rt-paths.ts vs rt-client/settings/paths.ts) test("both resolve HOME at call time, not module load", () => { process.env.HOME = "/tmp/parity-home-1"; - expect(clientPaths.userSettingsPath()).toBe(join("/tmp/parity-home-1", ".mattstack", "user", "settings.jsonc")); + expect(clientPaths.userSettingsPath()).toBe(join("/tmp/parity-home-1", ".mattstack", "user", "settings.user.jsonc")); process.env.HOME = "/tmp/parity-home-2"; - expect(clientPaths.userSettingsPath()).toBe(join("/tmp/parity-home-2", ".mattstack", "user", "settings.jsonc")); + expect(clientPaths.userSettingsPath()).toBe(join("/tmp/parity-home-2", ".mattstack", "user", "settings.user.jsonc")); expect(clientPaths.userSettingsPath()).toBe(rtPaths.userSettingsPath()); }); + + test("machineKey agrees between both modules, override file and hostname-slug paths alike", () => { + const home = mkdtempSync(join(tmpdir(), "parity-machine-key-")); + process.env.HOME = home; + + // No override file: both fall through to the same hostname() slug. + expect(clientPaths.machineKey()).toBe(rtPaths.machineKey()); + + // An override file: both read the same trimmed value. + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), " shared-override \n"); + expect(clientPaths.machineKey()).toBe("shared-override"); + expect(clientPaths.machineKey()).toBe(rtPaths.machineKey()); + + rmSync(home, { recursive: true, force: true }); + }); + + test("machineSettingsPath nests under user/local/ on both sides", () => { + process.env.HOME = "/tmp/parity-fake-home-2"; + expect(clientPaths.machineSettingsPath()).toBe( + join("/tmp/parity-fake-home-2", ".mattstack", "user", "local", clientPaths.machineKey(), "settings.local.jsonc"), + ); + expect(clientPaths.machineSettingsPath()).toBe(rtPaths.machineSettingsPath()); + }); }); diff --git a/lib/command-tree-def.ts b/lib/command-tree-def.ts index cdc380fc..1c3a23aa 100644 --- a/lib/command-tree-def.ts +++ b/lib/command-tree-def.ts @@ -599,7 +599,7 @@ export const TREE: Record = { args: [ { name: "Key", type: "text", placeholder: "rt.worktrees", hint: "Namespaced settings key (must be migrated:true)" }, { name: "Value", type: "text", placeholder: "{\"onDeck\":3}", hint: "JSON(C) value" }, - { name: "Scope", flag: "--scope", type: "select", hint: "Which store to write into", options: [{ value: "user", label: "user", hint: "~/.mattstack/user/settings.jsonc" }, { value: "team", label: "team", hint: "the local team clone's settings.jsonc" }, { value: "machine", label: "machine", hint: "~/.mattstack/settings.local.jsonc" }] }, + { name: "Scope", flag: "--scope", type: "select", hint: "Which store to write into", options: [{ value: "user", label: "user", hint: "~/.mattstack/user/settings.user.jsonc" }, { value: "team", label: "team", hint: "the local team clone's settings.team.jsonc" }, { value: "machine", label: "machine", hint: "~/.mattstack/user/local//settings.local.jsonc" }] }, { name: "Repo", flag: "--repo", type: "text", placeholder: "assured-dev", hint: "Repo name from ~/.mattstack/rt/repos.json — required for repo-scoped keys" }, { name: "Team", flag: "--team", type: "text", placeholder: "claimview", hint: "Which team's local store to write, for --scope team (only needed when several are cloned)" }, ], diff --git a/lib/daemon/__tests__/doppler-sync.test.ts b/lib/daemon/__tests__/doppler-sync.test.ts index 2d4a805f..edd3bce5 100644 --- a/lib/daemon/__tests__/doppler-sync.test.ts +++ b/lib/daemon/__tests__/doppler-sync.test.ts @@ -129,7 +129,7 @@ describe("reconcileForRepo", () => { // value with a warning before it ever reaches reconcileForRepo — so // this degrades the same way "nothing declared" does, honestly. const path = machineSettingsPath(); - mkdirSync(join(tmpHome, ".mattstack"), { recursive: true }); + mkdirSync(dirname(path), { recursive: true }); writeFileSync( path, JSON.stringify({ repos: { [IDENTITY]: { "rt.dopplerTemplate": { oops: true } } } }), diff --git a/lib/daemon/__tests__/worktree-reconciler.test.ts b/lib/daemon/__tests__/worktree-reconciler.test.ts index 989abbde..7197ef4e 100644 --- a/lib/daemon/__tests__/worktree-reconciler.test.ts +++ b/lib/daemon/__tests__/worktree-reconciler.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from "os"; import { basename, join } from "path"; import type { Logger } from "pino"; import { readJson, writeJson } from "../../json-store.ts"; -import { machineSettingsPath, rtDir } from "../../rt-paths.ts"; +import { machineSettingsPath, rtDir, teamSettingsPath } from "../../rt-paths.ts"; import { deriveRepoIdentity } from "../../settings/identity.ts"; import { findByPath, loadRegistry, saveRegistry, type TreeRecord } from "../../worktree/registry.ts"; import { @@ -361,7 +361,7 @@ describe("createWorktreeReconciler", () => { const manualPath = join(repo, ".worktrees", "manual"); execSync(`git worktree add -b manual-branch ${manualPath}`, { cwd: repo, shell: "/bin/zsh" }); - const teamStore = join(process.env.HOME!, ".mattstack", "teams", "claimview", "mattstack", "settings.jsonc"); + const teamStore = teamSettingsPath("claimview"); mkdirSync(join(teamStore, ".."), { recursive: true }); writeFileSync( teamStore, diff --git a/lib/rt-paths.ts b/lib/rt-paths.ts index 898aba6e..3b21538b 100644 --- a/lib/rt-paths.ts +++ b/lib/rt-paths.ts @@ -22,8 +22,8 @@ * a machine still carrying real legacy dirs. */ -import { existsSync, lstatSync, mkdirSync, renameSync } from "fs"; -import { homedir } from "os"; +import { existsSync, lstatSync, mkdirSync, readFileSync, renameSync } from "fs"; +import { homedir, hostname } from "os"; import { basename, join } from "path"; import { getSetting } from "./settings/resolve.ts"; @@ -67,38 +67,40 @@ export function repoDataDir(repoName: string): string { return join(reposDir(), repoName); } -// ─── Settings stores (RT-47) ────────────────────────────────────────────────── +// ─── Settings stores (RT-47, re-rooted under the home repo's user/ zone) ────── // -// These four paths live under ~/.mattstack directly, NOT under rtDir() — -// they are shared with the rest of mattstack (skills, board, deck), not just -// rt. The RT-46 source guards only police `.rt`/`rtDir()` reconstruction, so -// they don't apply here; these constructors exist purely for the -// one-layout-home rule (call-time HOME, single place that knows the path). +// These paths live under ~/.mattstack directly, NOT under rtDir() — they are +// shared with the rest of mattstack (skills, board, deck), not just rt. The +// RT-46 source guards only police `.rt`/`rtDir()` reconstruction, so they +// don't apply here; these constructors exist purely for the one-layout-home +// rule (call-time HOME, single place that knows the path). /** - * ~/.mattstack/user/settings.jsonc — the user store (in the mattstack-prefs - * repo): global keys plus `repos.` sections, scoped to this human - * across every machine they use. + * ~/.mattstack/user/settings.user.jsonc — the user store (in the home repo's + * tracked `user/` zone): global keys plus `repos.` sections, scoped + * to this human across every machine they use. */ export function userSettingsPath(): string { - return join(home(), ".mattstack", "user", "settings.jsonc"); + return join(home(), ".mattstack", "user", "settings.user.jsonc"); } /** - * ~/.mattstack/teams//mattstack/settings.jsonc — the team store (in the - * team repo zone): shared keys plus `repos.` sections. `team` is a - * team NAME (directory name under teamsDir()), not an identity. + * ~/.mattstack/teams//mattstack/settings.team.jsonc — the team store + * (in the team repo zone): shared keys plus `repos.` sections. + * `team` is a team NAME (directory name under teamsDir()), not an identity. */ export function teamSettingsPath(team: string): string { - return join(teamsDir(), team, "mattstack", "settings.jsonc"); + return join(teamsDir(), team, "mattstack", "settings.team.jsonc"); } /** - * ~/.mattstack/settings.local.jsonc — the machine store: local overrides, - * never committed or synced. The ONLY store where path literals are legal. + * ~/.mattstack/user/local//settings.local.jsonc — the machine + * store: local overrides, never committed or synced (`user/local/` is + * gitignored in the home repo). Nested per machine so multiple machines + * sharing the synced `user/` tree don't collide on one local-overrides file. */ export function machineSettingsPath(): string { - return join(home(), ".mattstack", "settings.local.jsonc"); + return join(home(), ".mattstack", "user", "local", machineKey(), "settings.local.jsonc"); } /** ~/.mattstack/teams — the container every team's local clone lives under. */ @@ -106,6 +108,36 @@ export function teamsDir(): string { return join(home(), ".mattstack", "teams"); } +/** + * The stable per-machine key that scopes the machine settings store — so + * `user/local//` never collides across machines sharing one synced + * `user/` tree. + * + * 1. `~/.mattstack/machine-key`, trimmed, if present and non-empty — an + * explicit override for machines whose hostname isn't stable or unique + * (fresh installs, cloned VMs). + * 2. Otherwise the hostname, slugified: lowercased, a trailing `.local` + * dropped (mDNS suffix, not part of the identity), every run of + * characters outside `[a-z0-9-]` collapsed to one `-`, leading/trailing + * `-` trimmed. An all-illegal hostname slugs to `""`, which falls back + * to `"default"` rather than producing an empty path segment. + */ +export function machineKey(): string { + const override = join(home(), ".mattstack", "machine-key"); + try { + const v = readFileSync(override, "utf8").trim(); + if (v) return v; + } catch { + // no override file — fall through to the hostname slug + } + const slug = hostname() + .toLowerCase() + .replace(/\.local$/, "") + .replace(/[^a-z0-9-]+/g, "-") + .replace(/^-+|-+$/g, ""); + return slug || "default"; +} + // ─── Tray app (MAT-383) ─────────────────────────────────────────────────────── // // The single source of truth for the tray app's on-disk names/paths, shared diff --git a/packages/rt-client/src/settings/__tests__/identity.test.ts b/packages/rt-client/src/settings/__tests__/identity.test.ts index 94f24d76..1f96bf58 100644 --- a/packages/rt-client/src/settings/__tests__/identity.test.ts +++ b/packages/rt-client/src/settings/__tests__/identity.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; -import { join } from "path"; +import { dirname, join } from "path"; import { runCapture } from "../exec.ts"; import { machineSettingsPath } from "../paths.ts"; import { normalizeRemote, identityFromRemote, deriveRepoIdentity, clearIdentityMemo } from "../identity.ts"; @@ -71,7 +71,7 @@ describe("settings/identity", () => { }); test("exact remote match in the machine store's rt.repoIdentityOverrides wins", () => { - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); writeFileSync( machineSettingsPath(), JSON.stringify({ @@ -86,7 +86,7 @@ describe("settings/identity", () => { }); test("override map present but remote not in it still falls through to normalizeRemote", () => { - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); writeFileSync( machineSettingsPath(), JSON.stringify({ @@ -132,7 +132,7 @@ describe("settings/identity", () => { test("routes through identityFromRemote so an override applies to derivation too", async () => { const dir = await initRepo("/private/tmp/some-local-remote"); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(machineSettingsPath()), { recursive: true }); writeFileSync( machineSettingsPath(), JSON.stringify({ diff --git a/packages/rt-client/src/settings/__tests__/paths.test.ts b/packages/rt-client/src/settings/__tests__/paths.test.ts new file mode 100644 index 00000000..eb8116c2 --- /dev/null +++ b/packages/rt-client/src/settings/__tests__/paths.test.ts @@ -0,0 +1,104 @@ +/** + * packages/rt-client/src/settings/paths.ts — the rt-client mirror of + * lib/rt-paths.ts's settings-store constructors (see that module's docblock + * for the "change there first, mirror here" convention). Parity between the + * two is covered separately by lib/__tests__/settings-paths-parity.test.ts; + * this file exercises the rt-client side's own behavior in isolation. + */ + +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; +import * as osReal from "os"; +import { tmpdir } from "os"; +import { join } from "path"; +import { machineKey, machineSettingsPath, teamSettingsPath, teamsDir, userSettingsPath } from "../paths.ts"; + +describe("settings/paths", () => { + const origHome = process.env.HOME; + afterEach(() => { + process.env.HOME = origHome; + }); + + describe("path shapes", () => { + test("userSettingsPath nests under user/settings.user.jsonc", () => { + process.env.HOME = "/tmp/fake-home-client-1"; + expect(userSettingsPath()).toBe("/tmp/fake-home-client-1/.mattstack/user/settings.user.jsonc"); + }); + + test("machineSettingsPath nests under user/local//settings.local.jsonc", () => { + const home = mkdtempSync(join(tmpdir(), "rt-client-machine-store-")); + process.env.HOME = home; + expect(machineSettingsPath()).toBe(join(home, ".mattstack", "user", "local", machineKey(), "settings.local.jsonc")); + rmSync(home, { recursive: true, force: true }); + }); + + test("teamSettingsPath nests under teams//mattstack/settings.team.jsonc", () => { + process.env.HOME = "/tmp/fake-home-client-2"; + expect(teamSettingsPath("acme")).toBe("/tmp/fake-home-client-2/.mattstack/teams/acme/mattstack/settings.team.jsonc"); + }); + + test("teamsDir resolves at call-time HOME", () => { + process.env.HOME = "/tmp/fake-home-client-3"; + expect(teamsDir()).toBe("/tmp/fake-home-client-3/.mattstack/teams"); + }); + }); + + describe("machineKey", () => { + const makeKeyHome = () => mkdtempSync(join(tmpdir(), "rt-client-machine-key-")); + + afterEach(() => { + mock.module("os", () => osReal); + }); + + test("an override file wins, trimmed", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), " my-custom-key \n"); + expect(machineKey()).toBe("my-custom-key"); + rmSync(home, { recursive: true, force: true }); + }); + + test("an override file that is empty after trim falls through to the hostname slug", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), " \n"); + mock.module("os", () => ({ ...osReal, hostname: () => "Real-Host" })); + expect(machineKey()).toBe("real-host"); + rmSync(home, { recursive: true, force: true }); + }); + + test("no override file at all: falls through to the hostname slug", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "Some-Host" })); + expect(machineKey()).toBe("some-host"); + rmSync(home, { recursive: true, force: true }); + }); + + test("hostname slug: lowercased, trailing .local stripped", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "Matts-MacBook-Pro.local" })); + expect(machineKey()).toBe("matts-macbook-pro"); + rmSync(home, { recursive: true, force: true }); + }); + + test("hostname slug: illegal characters collapse to single dashes, edges trimmed", () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => " weird_host!!name@@ " })); + expect(machineKey()).toBe("weird-host-name"); + rmSync(home, { recursive: true, force: true }); + }); + + test('hostname slug: an all-illegal hostname slugs to empty and falls back to "default"', () => { + const home = makeKeyHome(); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "!!!" })); + expect(machineKey()).toBe("default"); + rmSync(home, { recursive: true, force: true }); + }); + }); +}); diff --git a/packages/rt-client/src/settings/__tests__/stores.test.ts b/packages/rt-client/src/settings/__tests__/stores.test.ts index 3e070f16..c1920f73 100644 --- a/packages/rt-client/src/settings/__tests__/stores.test.ts +++ b/packages/rt-client/src/settings/__tests__/stores.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "fs"; import { tmpdir } from "os"; -import { join } from "path"; +import { dirname, join } from "path"; import { userSettingsPath, teamSettingsPath, teamsDir, machineSettingsPath } from "../paths.ts"; import { readStore, listTeams } from "../stores.ts"; @@ -55,7 +55,7 @@ describe("settings/stores", () => { test("global map never contains the repos key itself", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `{ "rt.foo": 1, "repos": {} }`); const store = readStore(file); @@ -76,7 +76,7 @@ describe("settings/stores", () => { test("malformed JSONC: exists true, empty maps, warns once", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `{ "rt.foo": ,,, this is not json`); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -95,7 +95,7 @@ describe("settings/stores", () => { test("a present but non-object \"repos\" value warns and degrades to empty repos, but global keys still parse", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `{ "rt.foo": 1, "repos": "not-an-object" }`); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -114,7 +114,7 @@ describe("settings/stores", () => { test("a present but array \"repos\" value also warns and degrades", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `{ "repos": [1, 2, 3] }`); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -130,7 +130,7 @@ describe("settings/stores", () => { test("an absent \"repos\" key is normal — no warn, empty repos", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `{ "rt.foo": 1 }`); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -146,7 +146,7 @@ describe("settings/stores", () => { test("a root that parses to a non-object (e.g. a bare array) is treated as malformed", () => { const file = machineSettingsPath(); - mkdirSync(join(home, ".mattstack"), { recursive: true }); + mkdirSync(dirname(file), { recursive: true }); writeFileSync(file, `[1, 2, 3]`); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -175,7 +175,7 @@ describe("settings/stores", () => { }); describe("listTeams", () => { - test("finds only team dirs that contain mattstack/settings.jsonc", () => { + test("finds only team dirs that contain mattstack/settings.team.jsonc", () => { // claimview: has a settings file. mkdirSync(join(home, ".mattstack", "teams", "claimview", "mattstack"), { recursive: true }); writeFileSync(teamSettingsPath("claimview"), "{}"); @@ -189,6 +189,13 @@ describe("settings/stores", () => { expect(listTeams().sort()).toEqual(["claimview"]); }); + test("a team dir with only the OLD-name settings.jsonc (no settings.team.jsonc) is NOT listed", () => { + mkdirSync(join(home, ".mattstack", "teams", "stale-team", "mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "teams", "stale-team", "mattstack", "settings.jsonc"), "{}"); + + expect(listTeams()).toEqual([]); + }); + test("no teams dir at all → empty list, no throw", () => { expect(listTeams()).toEqual([]); }); @@ -205,7 +212,7 @@ describe("settings/stores", () => { test("a symlinked team clone still counts as a team", () => { const real = join(home, "elsewhere", "claimview"); mkdirSync(join(real, "mattstack"), { recursive: true }); - writeFileSync(join(real, "mattstack", "settings.jsonc"), "{}"); + writeFileSync(join(real, "mattstack", "settings.team.jsonc"), "{}"); mkdirSync(teamsDir(), { recursive: true }); symlinkSync(real, join(teamsDir(), "claimview")); diff --git a/packages/rt-client/src/settings/__tests__/write.test.ts b/packages/rt-client/src/settings/__tests__/write.test.ts index 00f5caae..6b68c6c4 100644 --- a/packages/rt-client/src/settings/__tests__/write.test.ts +++ b/packages/rt-client/src/settings/__tests__/write.test.ts @@ -76,6 +76,18 @@ describe("settings/write", () => { expect(parsed["rt.worktrees"]).toEqual({ onDeck: 5 }); }); + test("creates the machine store's nested user/local/ directory when none of it exists yet", () => { + // The machine store now lives two directories deeper than a bare + // ~/.mattstack — nothing under ~/.mattstack/user/local exists on a + // fresh HOME, so the write must mkdir the whole chain, not just the + // immediate parent. + expect(() => setSetting("rt.worktrees", { onDeck: 7 }, "machine")).not.toThrow(); + + const content = readMachine(); + const parsed = JSON.parse(content.replace(/^\/\/.*\n/, "")); + expect(parsed["rt.worktrees"]).toEqual({ onDeck: 7 }); + }); + test("the header comment lands before the closing brace, not after it", () => { // Regression for the verified jsonc-parser footgun: modify() on a // comment-only file with no braces at all pushes the header AFTER the @@ -336,7 +348,7 @@ describe("settings/write", () => { const entries = readdirSync(dirname(userSettingsPath())); expect(entries.some((name) => name.endsWith(".tmp"))).toBe(false); - expect(entries).toContain("settings.jsonc"); + expect(entries).toContain("settings.user.jsonc"); }); test("a successful write's content matches what modify/applyEdits produced (no JSON.stringify round-trip)", () => { diff --git a/packages/rt-client/src/settings/paths.ts b/packages/rt-client/src/settings/paths.ts index 86ccf02d..8b7ebff9 100644 --- a/packages/rt-client/src/settings/paths.ts +++ b/packages/rt-client/src/settings/paths.ts @@ -9,29 +9,63 @@ * the original, so tests can repoint the whole tree at a temp dir. */ -import { homedir } from "os"; +import { readFileSync } from "fs"; +import { homedir, hostname } from "os"; import { join } from "path"; function home(): string { return process.env.HOME ?? homedir(); } -/** ~/.mattstack/user/settings.jsonc — the user store. */ +/** ~/.mattstack/user/settings.user.jsonc — the user store. */ export function userSettingsPath(): string { - return join(home(), ".mattstack", "user", "settings.jsonc"); + return join(home(), ".mattstack", "user", "settings.user.jsonc"); } -/** ~/.mattstack/teams//mattstack/settings.jsonc — the team store. */ +/** ~/.mattstack/teams//mattstack/settings.team.jsonc — the team store. */ export function teamSettingsPath(team: string): string { - return join(teamsDir(), team, "mattstack", "settings.jsonc"); + return join(teamsDir(), team, "mattstack", "settings.team.jsonc"); } -/** ~/.mattstack/settings.local.jsonc — the machine store (path literals legal here only). */ +/** + * ~/.mattstack/user/local//settings.local.jsonc — the machine + * store (path literals legal here only). + */ export function machineSettingsPath(): string { - return join(home(), ".mattstack", "settings.local.jsonc"); + return join(home(), ".mattstack", "user", "local", machineKey(), "settings.local.jsonc"); } /** ~/.mattstack/teams — the container every team's local clone lives under. */ export function teamsDir(): string { return join(home(), ".mattstack", "teams"); } + +/** + * The stable per-machine key that scopes the machine settings store — so + * `user/local//` never collides across machines sharing one synced + * `user/` tree. + * + * 1. `~/.mattstack/machine-key`, trimmed, if present and non-empty — an + * explicit override for machines whose hostname isn't stable or unique + * (fresh installs, cloned VMs). + * 2. Otherwise the hostname, slugified: lowercased, a trailing `.local` + * dropped (mDNS suffix, not part of the identity), every run of + * characters outside `[a-z0-9-]` collapsed to one `-`, leading/trailing + * `-` trimmed. An all-illegal hostname slugs to `""`, which falls back + * to `"default"` rather than producing an empty path segment. + */ +export function machineKey(): string { + const override = join(home(), ".mattstack", "machine-key"); + try { + const v = readFileSync(override, "utf8").trim(); + if (v) return v; + } catch { + // no override file — fall through to the hostname slug + } + const slug = hostname() + .toLowerCase() + .replace(/\.local$/, "") + .replace(/[^a-z0-9-]+/g, "-") + .replace(/^-+|-+$/g, ""); + return slug || "default"; +} diff --git a/packages/rt-client/src/settings/stores.ts b/packages/rt-client/src/settings/stores.ts index 4a590d9b..0f35cf57 100644 --- a/packages/rt-client/src/settings/stores.ts +++ b/packages/rt-client/src/settings/stores.ts @@ -82,7 +82,7 @@ export function readStore(file: string): StoreFile { /** * Names of every team that has a local settings store — i.e. subdirectories - * of teamsDir() that contain mattstack/settings.jsonc. A team dir without a + * of teamsDir() that contain mattstack/settings.team.jsonc. A team dir without a * settings file (a clone mid-setup, or an unrelated directory) is not yet a * team as far as the resolver is concerned. * From acd6b6cc1b8459350b7c5afbd8c76ee81efeecff Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 12:06:34 -0500 Subject: [PATCH 2/9] =?UTF-8?q?RT:=20home-repo=20reroot=20task=201,=20fix?= =?UTF-8?q?=20round=201=20=E2=80=94=20os=20mock=20restore=20+=20safe=20mac?= =?UTF-8?q?hine-key=20override?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The os mock restore was a no-op (mock.module mutates the live namespace in place, so restoring to the captured reference restored the mock to itself); capture the real hostname function before any test mocks os. Also tighten machineKey(): the override file's value must be a safe single path segment (no /, no \, not "." or "..") since it becomes a directory name directly under user/local/, otherwise fall through to the hostname slug. Co-Authored-By: Claude Fable 5 --- lib/__tests__/rt-paths.test.ts | 26 ++++++++++++++- lib/__tests__/settings-paths-parity.test.ts | 32 +++++++++++++++++-- lib/rt-paths.ts | 12 ++++--- .../src/settings/__tests__/paths.test.ts | 27 +++++++++++++++- packages/rt-client/src/settings/paths.ts | 12 ++++--- 5 files changed, 97 insertions(+), 12 deletions(-) diff --git a/lib/__tests__/rt-paths.test.ts b/lib/__tests__/rt-paths.test.ts index 0243c471..a8ef5f2f 100644 --- a/lib/__tests__/rt-paths.test.ts +++ b/lib/__tests__/rt-paths.test.ts @@ -19,6 +19,11 @@ import { } from "fs"; import * as osReal from "os"; import { tmpdir } from "os"; +// `mock.module` mutates the live "os" namespace object in place, so +// `osReal.hostname` itself becomes the mock the moment it's installed — +// restoring with `() => osReal` would restore the mock to itself. Capture +// the real function BEFORE any test can call mock.module("os", ...). +const realHostname = osReal.hostname; import { dirname, join } from "path"; import { rtDir, reposDir, repoDataDir, logsDir, @@ -97,7 +102,7 @@ describe("rt-paths", () => { const makeKeyHome = () => mkdtempSync(join(tmpdir(), "rt-paths-machine-key-")); afterEach(() => { - mock.module("os", () => osReal); + mock.module("os", () => ({ ...osReal, hostname: realHostname })); }); test("an override file wins, trimmed", () => { @@ -119,6 +124,25 @@ describe("rt-paths", () => { rmSync(home, { recursive: true, force: true }); }); + // An override becomes a directory name directly under user/local/, so a + // value that isn't a safe single path segment must not be honored — it + // would escape that directory (a separator) or resolve to a no-op/parent + // segment (".", "..") instead of a distinct machine's namespace. + test.each([ + ["a forward slash", "evil/key"], + ["a backslash", "evil\\key"], + ["exactly \".\"", "."], + ["exactly \"..\"", ".."], + ])("an override value containing %s is rejected — falls through to the hostname slug", (_label, unsafe) => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), unsafe); + mock.module("os", () => ({ ...osReal, hostname: () => "Safe-Host" })); + expect(machineKey()).toBe("safe-host"); + rmSync(home, { recursive: true, force: true }); + }); + test("no override file at all: falls through to the hostname slug", () => { const home = makeKeyHome(); process.env.HOME = home; diff --git a/lib/__tests__/settings-paths-parity.test.ts b/lib/__tests__/settings-paths-parity.test.ts index 98d93ae2..095603f7 100644 --- a/lib/__tests__/settings-paths-parity.test.ts +++ b/lib/__tests__/settings-paths-parity.test.ts @@ -7,17 +7,25 @@ * two callers onto different store paths. */ -import { describe, test, expect, afterEach } from "bun:test"; +import { describe, test, expect, afterEach, mock } from "bun:test"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"; +import * as osReal from "os"; import { tmpdir } from "os"; import { join } from "path"; import * as rtPaths from "../rt-paths.ts"; import * as clientPaths from "../../packages/rt-client/src/settings/paths.ts"; +// `mock.module` mutates the live "os" namespace object in place, so +// `osReal.hostname` itself becomes the mock the moment it's installed — +// restoring with `() => osReal` would restore the mock to itself. Capture +// the real function BEFORE any test can call mock.module("os", ...). +const realHostname = osReal.hostname; + describe("settings paths parity (lib/rt-paths.ts vs rt-client/settings/paths.ts)", () => { const origHome = process.env.HOME; afterEach(() => { process.env.HOME = origHome; + mock.module("os", () => ({ ...osReal, hostname: realHostname })); }); test("userSettingsPath/teamSettingsPath/machineSettingsPath/teamsDir agree under a faked HOME", () => { @@ -42,7 +50,12 @@ describe("settings paths parity (lib/rt-paths.ts vs rt-client/settings/paths.ts) const home = mkdtempSync(join(tmpdir(), "parity-machine-key-")); process.env.HOME = home; - // No override file: both fall through to the same hostname() slug. + // No override file: pin a realistic hostname on both sides so this arm + // exercises the full slug pipeline (not just "both happen to agree"), + // regardless of what the real machine's hostname is or file run order. + mock.module("os", () => ({ ...osReal, hostname: () => "Matts-MacBook-Pro.local" })); + expect(clientPaths.machineKey()).toBe("matts-macbook-pro"); + expect(rtPaths.machineKey()).toBe("matts-macbook-pro"); expect(clientPaths.machineKey()).toBe(rtPaths.machineKey()); // An override file: both read the same trimmed value. @@ -54,6 +67,21 @@ describe("settings paths parity (lib/rt-paths.ts vs rt-client/settings/paths.ts) rmSync(home, { recursive: true, force: true }); }); + test("an unsafe override value (path separator, \".\", or \"..\") is rejected on both sides alike", () => { + const home = mkdtempSync(join(tmpdir(), "parity-machine-key-unsafe-")); + process.env.HOME = home; + mock.module("os", () => ({ ...osReal, hostname: () => "Safe-Host" })); + mkdirSync(join(home, ".mattstack"), { recursive: true }); + + for (const unsafe of ["evil/key", "evil\\key", ".", ".."]) { + writeFileSync(join(home, ".mattstack", "machine-key"), unsafe); + expect(clientPaths.machineKey()).toBe("safe-host"); + expect(rtPaths.machineKey()).toBe("safe-host"); + } + + rmSync(home, { recursive: true, force: true }); + }); + test("machineSettingsPath nests under user/local/ on both sides", () => { process.env.HOME = "/tmp/parity-fake-home-2"; expect(clientPaths.machineSettingsPath()).toBe( diff --git a/lib/rt-paths.ts b/lib/rt-paths.ts index 3b21538b..b24fcf90 100644 --- a/lib/rt-paths.ts +++ b/lib/rt-paths.ts @@ -113,9 +113,13 @@ export function teamsDir(): string { * `user/local//` never collides across machines sharing one synced * `user/` tree. * - * 1. `~/.mattstack/machine-key`, trimmed, if present and non-empty — an - * explicit override for machines whose hostname isn't stable or unique - * (fresh installs, cloned VMs). + * 1. `~/.mattstack/machine-key`, trimmed, if present, non-empty, and a SAFE + * PATH SEGMENT (no `/` or `\`, not `.` or `..`) — an explicit override + * for machines whose hostname isn't stable or unique (fresh installs, + * cloned VMs). The value becomes a directory name directly under + * `user/local/`, so anything else (a separator, or a segment that would + * walk up/stay put) is treated exactly as if the file were absent, + * rather than let the override escape that directory. * 2. Otherwise the hostname, slugified: lowercased, a trailing `.local` * dropped (mDNS suffix, not part of the identity), every run of * characters outside `[a-z0-9-]` collapsed to one `-`, leading/trailing @@ -126,7 +130,7 @@ export function machineKey(): string { const override = join(home(), ".mattstack", "machine-key"); try { const v = readFileSync(override, "utf8").trim(); - if (v) return v; + if (v && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\")) return v; } catch { // no override file — fall through to the hostname slug } diff --git a/packages/rt-client/src/settings/__tests__/paths.test.ts b/packages/rt-client/src/settings/__tests__/paths.test.ts index eb8116c2..9598b007 100644 --- a/packages/rt-client/src/settings/__tests__/paths.test.ts +++ b/packages/rt-client/src/settings/__tests__/paths.test.ts @@ -13,6 +13,12 @@ import { tmpdir } from "os"; import { join } from "path"; import { machineKey, machineSettingsPath, teamSettingsPath, teamsDir, userSettingsPath } from "../paths.ts"; +// `mock.module` mutates the live "os" namespace object in place, so +// `osReal.hostname` itself becomes the mock the moment it's installed — +// restoring with `() => osReal` would restore the mock to itself. Capture +// the real function BEFORE any test can call mock.module("os", ...). +const realHostname = osReal.hostname; + describe("settings/paths", () => { const origHome = process.env.HOME; afterEach(() => { @@ -47,7 +53,7 @@ describe("settings/paths", () => { const makeKeyHome = () => mkdtempSync(join(tmpdir(), "rt-client-machine-key-")); afterEach(() => { - mock.module("os", () => osReal); + mock.module("os", () => ({ ...osReal, hostname: realHostname })); }); test("an override file wins, trimmed", () => { @@ -69,6 +75,25 @@ describe("settings/paths", () => { rmSync(home, { recursive: true, force: true }); }); + // An override becomes a directory name directly under user/local/, so a + // value that isn't a safe single path segment must not be honored — it + // would escape that directory (a separator) or resolve to a no-op/parent + // segment (".", "..") instead of a distinct machine's namespace. + test.each([ + ["a forward slash", "evil/key"], + ["a backslash", "evil\\key"], + ["exactly \".\"", "."], + ["exactly \"..\"", ".."], + ])("an override value containing %s is rejected — falls through to the hostname slug", (_label, unsafe) => { + const home = makeKeyHome(); + process.env.HOME = home; + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), unsafe); + mock.module("os", () => ({ ...osReal, hostname: () => "Safe-Host" })); + expect(machineKey()).toBe("safe-host"); + rmSync(home, { recursive: true, force: true }); + }); + test("no override file at all: falls through to the hostname slug", () => { const home = makeKeyHome(); process.env.HOME = home; diff --git a/packages/rt-client/src/settings/paths.ts b/packages/rt-client/src/settings/paths.ts index 8b7ebff9..d42044e7 100644 --- a/packages/rt-client/src/settings/paths.ts +++ b/packages/rt-client/src/settings/paths.ts @@ -45,9 +45,13 @@ export function teamsDir(): string { * `user/local//` never collides across machines sharing one synced * `user/` tree. * - * 1. `~/.mattstack/machine-key`, trimmed, if present and non-empty — an - * explicit override for machines whose hostname isn't stable or unique - * (fresh installs, cloned VMs). + * 1. `~/.mattstack/machine-key`, trimmed, if present, non-empty, and a SAFE + * PATH SEGMENT (no `/` or `\`, not `.` or `..`) — an explicit override + * for machines whose hostname isn't stable or unique (fresh installs, + * cloned VMs). The value becomes a directory name directly under + * `user/local/`, so anything else (a separator, or a segment that would + * walk up/stay put) is treated exactly as if the file were absent, + * rather than let the override escape that directory. * 2. Otherwise the hostname, slugified: lowercased, a trailing `.local` * dropped (mDNS suffix, not part of the identity), every run of * characters outside `[a-z0-9-]` collapsed to one `-`, leading/trailing @@ -58,7 +62,7 @@ export function machineKey(): string { const override = join(home(), ".mattstack", "machine-key"); try { const v = readFileSync(override, "utf8").trim(); - if (v) return v; + if (v && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\")) return v; } catch { // no override file — fall through to the hostname slug } From 32f6a4ad62d0409647b586928f4228e8a764a50a Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 12:32:57 -0500 Subject: [PATCH 3/9] RT: sops triple moves to user/-rooted (path_regex, spawn cwd, filename-override) renderSopsYaml's path_regex, buildSecretsSpawnOptions' cwd pin, and encryptDomain's --filename-override all move from root-of-mattstackHome to /user in lockstep: sops resolves .sops.yaml and matches path_regex relative to cwd, so if only some of the three moved, sops would silently match no creation rule. commands/home.ts's .sops.yaml read/write path and its git-add hint move to user/.sops.yaml to match. Co-Authored-By: Claude Fable 5 --- commands/__tests__/home.test.ts | 2 +- commands/home.ts | 7 +++++-- lib/home/__tests__/age-key.test.ts | 4 ++-- lib/home/age-key.ts | 9 +++++++-- lib/secrets/__tests__/store.test.ts | 10 +++++----- lib/secrets/store.ts | 25 ++++++++++++++++--------- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/commands/__tests__/home.test.ts b/commands/__tests__/home.test.ts index d57b9e15..02b201ec 100644 --- a/commands/__tests__/home.test.ts +++ b/commands/__tests__/home.test.ts @@ -8,7 +8,7 @@ import { join } from "path"; const FAKE_PUBLIC_KEY = "age1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"; const FAKE_PRIVATE_KEY = "AGE-SECRET-KEY-1QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ"; -const SOPS_YAML_PATH = join(mattstackHome(), ".sops.yaml"); +const SOPS_YAML_PATH = join(mattstackHome(), "user", ".sops.yaml"); /** In-memory .sops.yaml — never touches the real filesystem. */ class FakeSopsYamlSeam implements SopsYamlSeam { diff --git a/commands/home.ts b/commands/home.ts index cddb9103..c641374b 100644 --- a/commands/home.ts +++ b/commands/home.ts @@ -139,13 +139,16 @@ function describeStep(step: InitStep): string { async function ensureHomeAgeKey(seams: AgeKeySeam, sopsYamlSeam: SopsYamlSeam = defaultSopsYamlSeam()): Promise { const { publicKey } = await ensureAgeKey(seams); - const sopsYamlPath = join(mattstackHome(), ".sops.yaml"); + // Lives under user/ (not the repo root): sops matches path_regex cwd-relative + // and every sops spawn pins cwd to /user (store.ts), so + // .sops.yaml must sit there too for that discovery to find it. + const sopsYamlPath = join(mattstackHome(), "user", ".sops.yaml"); const existing = sopsYamlSeam.read(sopsYamlPath); if (existing === null || sopsYamlRecipient(existing) !== publicKey) { sopsYamlSeam.write(sopsYamlPath, renderSopsYaml(publicKey)); console.log( `rt home init: wrote ${sopsYamlPath} (recipient ${publicKey}) — it's tracked, so commit it:\n` + - ` git -C ${mattstackHome()} add .sops.yaml && git -C ${mattstackHome()} commit -m "home: sops recipient"`, + ` git -C ${mattstackHome()} add user/.sops.yaml && git -C ${mattstackHome()} commit -m "home: sops recipient"`, ); } diff --git a/lib/home/__tests__/age-key.test.ts b/lib/home/__tests__/age-key.test.ts index 000a7249..3adee3f9 100644 --- a/lib/home/__tests__/age-key.test.ts +++ b/lib/home/__tests__/age-key.test.ts @@ -203,9 +203,9 @@ describe("withArgvRedaction", () => { }); describe("renderSopsYaml", () => { - test("emits a creation rule encrypting user/secrets/** to the given recipient", () => { + test("emits a creation rule encrypting secrets/** (cwd-relative, cwd pinned to /user) to the given recipient", () => { const yaml = renderSopsYaml("age1xyz"); - expect(yaml).toContain("path_regex: user/secrets/.*"); + expect(yaml).toContain("path_regex: secrets/.*"); expect(yaml).toContain("age1xyz"); }); }); diff --git a/lib/home/age-key.ts b/lib/home/age-key.ts index e3fa906b..630ac9d9 100644 --- a/lib/home/age-key.ts +++ b/lib/home/age-key.ts @@ -1,7 +1,12 @@ /** * The mattstack age key: one identity, custodied in the macOS keychain, - * never written to any file. Every secret path under the home repo's + * never written to any file. Every secret under the home repo's * `user/secrets/` encrypts to its public recipient (see renderSopsYaml). + * renderSopsYaml's `path_regex` is `secrets/.*`, not `user/secrets/.*`: sops + * matches it against the filename relative to cwd, and every sops spawn + * pins cwd to `/user` (lib/secrets/store.ts) — the regex, + * the cwd pin, and the `--filename-override` all move together or sops + * silently matches no rule. * * All keychain/age-keygen calls route through the injected AgeKeySeam so * tests never touch the real keychain. The private key crosses process @@ -120,7 +125,7 @@ export async function ensureAgeKey(seams: AgeKeySeam): Promise<{ publicKey: stri } export function renderSopsYaml(publicKey: string): string { - return ["creation_rules:", " - path_regex: user/secrets/.*", ` age: ${publicKey}`, ""].join("\n"); + return ["creation_rules:", " - path_regex: secrets/.*", ` age: ${publicKey}`, ""].join("\n"); } /** The inverse of renderSopsYaml: the `age:` recipient from a rendered .sops.yaml, or null if the shape doesn't match (a hand-edited file with no recognizable recipient line). */ diff --git a/lib/secrets/__tests__/store.test.ts b/lib/secrets/__tests__/store.test.ts index 21b52b3c..b93746c2 100644 --- a/lib/secrets/__tests__/store.test.ts +++ b/lib/secrets/__tests__/store.test.ts @@ -302,7 +302,7 @@ describe("writeSecret", () => { expect(execSeam.calls.map((c) => c.cmd)).toEqual([ ["sops", "-d", path], - ["sops", "-e", "--filename-override", join("user", "secrets", `${domain}.json`), "--output", outputTmp, staging], + ["sops", "-e", "--filename-override", join("secrets", `${domain}.json`), "--output", outputTmp, staging], ["sops", "-d", outputTmp], ]); // The round-trip readback carries the same SOPS_AGE_KEY env as any other sops call — never argv. @@ -331,7 +331,7 @@ describe("writeSecret", () => { const outputTmp = `${path}.${process.pid}.tmp`; expect(execSeam.calls.map((c) => c.cmd)).toEqual([ - ["sops", "-e", "--filename-override", join("user", "secrets", `${domain}.json`), "--output", outputTmp, stagingPath(domain)], + ["sops", "-e", "--filename-override", join("secrets", `${domain}.json`), "--output", outputTmp, stagingPath(domain)], ["sops", "-d", outputTmp], ]); expect(execSeam.files.get(path)).toBe(DEFAULT_CIPHERTEXT); @@ -550,14 +550,14 @@ describe("rotateSecret", () => { }); describe("real seam spawn options — cwd pin (Task 5 carried review item)", () => { - test("pins cwd to mattstackHome() so sops resolves THIS home's .sops.yaml, never a foreign cwd's", () => { + test("pins cwd to /user so sops resolves THIS home's .sops.yaml (and secrets/.* regex), never a foreign cwd's", () => { const opts = buildSecretsSpawnOptions(); - expect(opts.cwd).toBe(mattstackHome()); + expect(opts.cwd).toBe(join(mattstackHome(), "user")); }); test("still layers opts.env (e.g. SOPS_AGE_KEY) over process.env alongside the cwd pin", () => { const opts = buildSecretsSpawnOptions({ env: { SOPS_AGE_KEY: "age-secret-key-test" } }); - expect(opts.cwd).toBe(mattstackHome()); + expect(opts.cwd).toBe(join(mattstackHome(), "user")); expect(opts.env.SOPS_AGE_KEY).toBe("age-secret-key-test"); }); }); diff --git a/lib/secrets/store.ts b/lib/secrets/store.ts index 540c5d7a..fa37fdca 100644 --- a/lib/secrets/store.ts +++ b/lib/secrets/store.ts @@ -3,12 +3,17 @@ * decrypted with the mattstack age key (lib/home/age-key.ts) via SOPS_AGE_KEY. * The key crosses into the sops subprocess ONLY through that env var — never * argv, never a file — mirroring readAgeKey's own custody rule and its - * `.sops.yaml` creation rule for `user/secrets/**` (lib/home/age-key.ts). + * `.sops.yaml` creation rule for `secrets/**` (lib/home/age-key.ts). sops + * resolves `.sops.yaml` and matches `path_regex` cwd-relative, so every sops + * spawn pins cwd to `/user` (buildSecretsSpawnOptions) — the + * regex, the cwd pin, and the `--filename-override` below all move together + * or sops silently matches no rule and encrypts to the wrong recipient. * * Write idiom: stage plaintext at `~/.mattstack/rt/tmp/..json` * (rt/ is gitignored — never a tracked path), fsync it, encrypt with - * `--filename-override user/secrets/.json` (keeps the `.sops.yaml` - * path_regex matching even though the real input lives in rt/tmp) into + * `--filename-override secrets/.json` (relative to the pinned cwd, + * keeping the `.sops.yaml` path_regex matching even though the real input + * lives in rt/tmp) into * `..tmp` (pid-qualified so two concurrent writers can't * unlink each other's tmp output), decrypt that tmp output and confirm the * newly written key round-trips — a real check that the encrypt used the @@ -219,9 +224,9 @@ async function encryptDomain( const stagingPath = join(stagingDir, `${domain}.${process.pid}.json`); const outputTmpPath = `${targetPath}.${process.pid}.tmp`; - // Relative to the home root, matching .sops.yaml's `path_regex: - // user/secrets/.*` — the real input path (under rt/tmp) would never match. - const filenameOverride = join("user", "secrets", `${domain}.json`); + // Relative to the pinned cwd (/user), matching .sops.yaml's + // `path_regex: secrets/.*` — the real input path (under rt/tmp) would never match. + const filenameOverride = join("secrets", `${domain}.json`); try { execSeam.writeFile(stagingPath, JSON.stringify(payload, null, 2)); @@ -334,8 +339,10 @@ function debugLog(cmd: string[], sensitive: boolean | undefined): void { * `.sops.yaml` cwd-relative, so a spawn from a foreign cwd (e.g. a CLI * command invoked from inside some other repo) can silently match that * repo's own `.sops.yaml` rules and encrypt to the wrong recipients instead - * of erroring. Pinning `cwd` to `mattstackHome()` makes every sops call - * resolve the home repo's rules regardless of the caller's cwd. + * of erroring. Pinning `cwd` to `/user` makes every sops call + * resolve the home repo's `.sops.yaml` (also under `user/`) and its + * `secrets/.*` path_regex regardless of the caller's cwd — the regex is + * cwd-relative, not root-relative, so this cwd and that regex move together. */ export function buildSecretsSpawnOptions(opts?: { env?: Record }): { cwd: string; @@ -344,7 +351,7 @@ export function buildSecretsSpawnOptions(opts?: { env?: Record } stderr: "pipe"; } { return { - cwd: mattstackHome(), + cwd: join(mattstackHome(), "user"), // A fresh object every call (not a live reference/pass-through like // init-exec.ts's raw `env: process.env`) — but since it's built from // process.env at call time rather than cached once at module load, a From fd73fabf7df14ad84b433540ccf080d4c61ada51 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 13:06:04 -0500 Subject: [PATCH 4/9] =?UTF-8?q?RT:=20rt=20home=20init=20rewrite=20?= =?UTF-8?q?=E2=80=94=20clone=20user/=20+=20provision=20this=20machine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retires the old adopt-a-root-repo flow (createRepo/gitInit/foldInPrefs/ unlinkUserClone/adoptCommit, prefsRemoteUrl/hasUserClone) in favor of the re-rooted model: user/ IS the personal repo, cloned from --url (default mattstack-home), and init's job becomes per-machine provisioning (state dirs, machine-key file, local// profile dir, the skills.jsonc compat symlink). Each step is gated on its own HomeState probe, so a fully-provisioned machine converges to an empty plan without a special -cased short-circuit. boundary.ts's HOME_BOUNDARY becomes the user-repo's own gitignore hygiene list (.DS_Store, *.sock, *.tmp — no local/); writeGitignore/writeOwners now write into user/ and only run alongside a fresh clone, since an already-cloned repo already carries them from its own history. Also renames a describe block in secrets store.test.ts that named a task number instead of the behavior under test. --- commands/__tests__/home.test.ts | 258 +++++++++--------- commands/home.ts | 189 ++++++------- lib/command-tree-def.ts | 10 +- lib/home/__tests__/boundary.test.ts | 121 +-------- lib/home/__tests__/init-exec.test.ts | 381 +++++++-------------------- lib/home/__tests__/init-plan.test.ts | 193 ++++++++------ lib/home/boundary.ts | 35 +-- lib/home/init-exec.ts | 141 +++------- lib/home/init-plan.ts | 104 ++++---- lib/secrets/__tests__/store.test.ts | 2 +- 10 files changed, 520 insertions(+), 914 deletions(-) diff --git a/commands/__tests__/home.test.ts b/commands/__tests__/home.test.ts index 02b201ec..6daa994e 100644 --- a/commands/__tests__/home.test.ts +++ b/commands/__tests__/home.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, spyOn } from "bun:test"; -import { gatherHomeState, homeInit, type HomeProbes, type SopsYamlSeam } from "../home.ts"; -import { buildInitPlan } from "../../lib/home/init-plan.ts"; +import { DEFAULT_USER_REPO_URL, gatherHomeState, homeInit, type HomeProbes, type SopsYamlSeam } from "../home.ts"; +import { STATE_DIR_NAMES } from "../../lib/home/init-plan.ts"; import type { ExecResult, ExecSeam } from "../../lib/home/init-exec.ts"; import { renderSopsYaml, type AgeExecResult, type AgeKeySeam } from "../../lib/home/age-key.ts"; import { mattstackHome } from "../../lib/rt-paths.ts"; @@ -9,6 +9,7 @@ import { join } from "path"; const FAKE_PUBLIC_KEY = "age1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"; const FAKE_PRIVATE_KEY = "AGE-SECRET-KEY-1QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ"; const SOPS_YAML_PATH = join(mattstackHome(), "user", ".sops.yaml"); +const KEY = "mbp-14"; /** In-memory .sops.yaml — never touches the real filesystem. */ class FakeSopsYamlSeam implements SopsYamlSeam { @@ -50,91 +51,87 @@ function fakeProbes(overrides: Partial): HomeProbes { return { isGitRepo: () => false, exists: () => false, - listTeamClones: () => [], - readFile: () => null, + readSymlinkTarget: () => null, ...overrides, }; } +const FULLY_PROVISIONED_PROBES = (): HomeProbes => + fakeProbes({ + isGitRepo: (dir) => dir.endsWith("/user"), + exists: () => true, + readSymlinkTarget: (path) => (path.endsWith("/skills.jsonc") ? join("user", "skills.jsonc") : null), + }); + describe("gatherHomeState", () => { - test("hasUserClone is true only when user/ is itself a git clone", () => { - const probes = fakeProbes({ - isGitRepo: (dir) => dir.endsWith("/user"), - }); - const state = gatherHomeState("/home", probes); - expect(state.hasUserClone).toBe(true); + test("userRepoPresent is true only when user/ is itself a git clone", () => { + const probes = fakeProbes({ isGitRepo: (dir) => dir.endsWith("/user") }); + const state = gatherHomeState("/home", probes, KEY); + expect(state.userRepoPresent).toBe(true); }); - test("a plain (non-git) user/ directory does not count as a clone, and yields no foldInPrefs step", () => { - const probes = fakeProbes({ - // user/ exists on disk but isn't a git repo — e.g. a half-materialized - // or manually-created directory, not the mattstack-prefs clone. - exists: (path) => path.endsWith("/user"), - isGitRepo: () => false, - }); - const state = gatherHomeState("/home", probes); - expect(state.hasUserClone).toBe(false); + test("a plain (non-git) user/ directory does not count as a clone", () => { + const probes = fakeProbes({ exists: (path) => path.endsWith("/user"), isGitRepo: () => false }); + const state = gatherHomeState("/home", probes, KEY); + expect(state.userRepoPresent).toBe(false); + }); - const plan = buildInitPlan(state); - expect(plan.steps.map((s) => s.kind)).not.toContain("foldInPrefs"); + test("machineKeyFilePresent reflects the root machine-key file only", () => { + const probes = fakeProbes({ exists: (path) => path === "/home/machine-key" }); + const state = gatherHomeState("/home", probes, KEY); + expect(state.machineKeyFilePresent).toBe(true); }); - test("prefsRemoteUrl is parsed from user/.git/config while the clone still exists", () => { - const probes = fakeProbes({ - isGitRepo: (dir) => dir.endsWith("/user"), - readFile: (path) => - path.endsWith("/user/.git/config") - ? '[remote "origin"]\n\turl = https://github.com/mattgoodwin/mattstack-prefs.git\n' - : null, - }); - const state = gatherHomeState("/home", probes); - expect(state.prefsRemoteUrl).toBe("https://github.com/mattgoodwin/mattstack-prefs.git"); + test("profileDirPresent checks user/local//, not just any local dir", () => { + const probes = fakeProbes({ exists: (path) => path === join("/home", "user", "local", KEY) }); + const state = gatherHomeState("/home", probes, KEY); + expect(state.profileDirPresent).toBe(true); }); - test("prefsRemoteUrl is undefined when there is no user clone, even if readFile would return something", () => { - const probes = fakeProbes({ - isGitRepo: () => false, - readFile: () => '[remote "origin"]\n\turl = https://example.com/should-not-be-read.git\n', - }); - const state = gatherHomeState("/home", probes); - expect(state.prefsRemoteUrl).toBeUndefined(); + test("skillsSymlinkPresent is true only when the symlink target is exactly user/skills.jsonc", () => { + const correct = fakeProbes({ readSymlinkTarget: () => join("user", "skills.jsonc") }); + expect(gatherHomeState("/home", correct, KEY).skillsSymlinkPresent).toBe(true); + + const wrong = fakeProbes({ readSymlinkTarget: () => "/some/other/path" }); + expect(gatherHomeState("/home", wrong, KEY).skillsSymlinkPresent).toBe(false); }); - test("prefsRemoteUrl is undefined when the config can't be read or parsed", () => { - const probes = fakeProbes({ - isGitRepo: (dir) => dir.endsWith("/user"), - readFile: () => null, - }); - const state = gatherHomeState("/home", probes); - expect(state.prefsRemoteUrl).toBeUndefined(); + test("skillsSymlinkBlocked is true only when a REAL file (not a symlink) sits at the root path", () => { + const realFile = fakeProbes({ readSymlinkTarget: () => null, exists: (path) => path.endsWith("/skills.jsonc") }); + expect(gatherHomeState("/home", realFile, KEY).skillsSymlinkBlocked).toBe(true); - const plan = buildInitPlan(state); - expect(plan.steps).toEqual([]); - expect(plan.reason).toBe("prefs-remote-unreadable"); + const absent = fakeProbes({ readSymlinkTarget: () => null, exists: () => false }); + expect(gatherHomeState("/home", absent, KEY).skillsSymlinkBlocked).toBe(false); + + const validSymlink = fakeProbes({ readSymlinkTarget: () => join("user", "skills.jsonc") }); + expect(gatherHomeState("/home", validSymlink, KEY).skillsSymlinkBlocked).toBe(false); + }); + + test("stateDirsMissing lists only the state dirs absent under home", () => { + const probes = fakeProbes({ exists: (path) => path === "/home/rt" || path === "/home/deck" }); + const state = gatherHomeState("/home", probes, KEY); + expect(state.stateDirsMissing).toEqual(STATE_DIR_NAMES.filter((n) => n !== "rt" && n !== "deck")); }); }); -/** Records argv only; used to prove preflight/idempotence run zero real steps. */ +/** Records argv only; used to prove init-step execution and idempotence. */ class FakeSeam implements ExecSeam { - calls: string[][] = []; - constructor( - private opts: { - failRun?: (cmd: string[]) => boolean; - throwOn?: (cmd: string[]) => boolean; - stdout?: (cmd: string[]) => string; - } = {}, - ) {} + calls: { kind: string; arg: unknown }[] = []; + constructor(private opts: { failRun?: (cmd: string[]) => boolean } = {}) {} async run(cmd: string[]): Promise { - this.calls.push(cmd); - if (this.opts.throwOn?.(cmd)) throw new Error(`spawn ${cmd[0]} ENOENT`); + this.calls.push({ kind: "run", arg: cmd }); if (this.opts.failRun?.(cmd)) return { code: 1, stdout: "", stderr: "boom" }; - return { code: 0, stdout: this.opts.stdout?.(cmd) ?? "", stderr: "" }; + return { code: 0, stdout: "", stderr: "" }; + } + async writeFile(path: string, content: string): Promise { + this.calls.push({ kind: "writeFile", arg: { path, content } }); + } + async mkdirp(path: string): Promise { + this.calls.push({ kind: "mkdirp", arg: path }); } - async writeFile(): Promise {} - async removeDir(): Promise {} - async mkTempDir(): Promise { - return "/tmp/rt-home-fold-test"; + async writeSymlink(path: string, target: string): Promise { + this.calls.push({ kind: "writeSymlink", arg: { path, target } }); } } @@ -145,21 +142,25 @@ async function runHomeInit( ageKeySeam: AgeKeySeam = new FakeAgeKeySeam(), args: string[] = [], sopsYamlSeam: SopsYamlSeam = new FakeSopsYamlSeam(), -): Promise<{ exitCode: number | undefined; logs: string[] }> { + key: string = KEY, +): Promise<{ exitCode: number | undefined; logs: string[]; errors: string[] }> { const exitSpy = spyOn(process, "exit").mockImplementation(() => { throw new Error("process.exit"); }); const logs: string[] = []; + const errors: string[] = []; spyOn(console, "log").mockImplementation((...parts: unknown[]) => { logs.push(parts.map(String).join(" ")); }); - spyOn(console, "error").mockImplementation(() => {}); + spyOn(console, "error").mockImplementation((...parts: unknown[]) => { + errors.push(parts.map(String).join(" ")); + }); try { - await homeInit(args, {}, probes, exec, ageKeySeam, sopsYamlSeam); - return { exitCode: undefined, logs }; + await homeInit(args, {}, probes, exec, ageKeySeam, sopsYamlSeam, key); + return { exitCode: undefined, logs, errors }; } catch { const code = exitSpy.mock.calls.at(-1)?.[0] as number | undefined; - return { exitCode: code, logs }; + return { exitCode: code, logs, errors }; } finally { exitSpy.mockRestore(); (console.log as unknown as { mockRestore: () => void }).mockRestore(); @@ -168,125 +169,88 @@ async function runHomeInit( } describe("homeInit", () => { - test("already-initialized: exits cleanly, runs no preflight or step, but still ensures the age key (idempotent)", async () => { + test("fully provisioned: runs no step, but still ensures the age key (idempotent)", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); - const { exitCode } = await runHomeInit(fakeProbes({ isGitRepo: () => true }), seam, ageKeySeam); + const { exitCode } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam); expect(exitCode).toBeUndefined(); expect(seam.calls).toEqual([]); expect(ageKeySeam.calls.some((c) => c[1] === "find-generic-password")).toBe(true); }); - test("already-initialized: backfills .sops.yaml when it's missing (a home repo that predates this step)", async () => { + test("fully provisioned: backfills .sops.yaml when it's missing (a home repo that predates this step)", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); const sopsYamlSeam = new FakeSopsYamlSeam(); - await runHomeInit(fakeProbes({ isGitRepo: () => true }), seam, ageKeySeam, [], sopsYamlSeam); + await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); expect(sopsYamlSeam.files.get(SOPS_YAML_PATH)).toBe(renderSopsYaml(FAKE_PUBLIC_KEY)); }); - test("already-initialized: an existing .sops.yaml with the current key's recipient is left untouched", async () => { + test("fully provisioned: an existing .sops.yaml with the current key's recipient is left untouched", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml(FAKE_PUBLIC_KEY) }); - await runHomeInit(fakeProbes({ isGitRepo: () => true }), seam, ageKeySeam, [], sopsYamlSeam); + await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); expect(sopsYamlSeam.writes).toEqual([]); }); - test("already-initialized: an existing .sops.yaml with a stale recipient (key rotation) is rewritten", async () => { + test("fully provisioned: an existing .sops.yaml with a stale recipient (key rotation) is rewritten", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml("age1stale") }); - await runHomeInit(fakeProbes({ isGitRepo: () => true }), seam, ageKeySeam, [], sopsYamlSeam); + await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); expect(sopsYamlSeam.files.get(SOPS_YAML_PATH)).toBe(renderSopsYaml(FAKE_PUBLIC_KEY)); }); - test("already-initialized --dry-run: never touches the age key either", async () => { + test("fully provisioned --dry-run: never touches the age key either", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); const sopsYamlSeam = new FakeSopsYamlSeam(); - const { exitCode } = await runHomeInit(fakeProbes({ isGitRepo: () => true }), seam, ageKeySeam, ["--dry-run"], sopsYamlSeam); + const { exitCode } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, ["--dry-run"], sopsYamlSeam); expect(exitCode).toBeUndefined(); expect(ageKeySeam.calls).toEqual([]); expect(sopsYamlSeam.writes).toEqual([]); }); - test("prefs-remote-unreadable: exits 1 and runs no preflight, init step, or age-key call", async () => { + test("a fresh, fully successful init clones with the default URL, mints the age key, and writes .sops.yaml after adoption", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); - const probes = fakeProbes({ - isGitRepo: (dir) => dir.endsWith("/user"), // hasUserClone, home itself is not a repo - readFile: () => null, // config unreadable -> prefsRemoteUrl stays undefined - }); - const { exitCode } = await runHomeInit(probes, seam, ageKeySeam); - - expect(exitCode).toBe(1); - expect(seam.calls).toEqual([]); - expect(ageKeySeam.calls).toEqual([]); - }); - - test("preflight failure (gh not authenticated) prints a hint, runs no init step, and never touches the age key", async () => { - const seam = new FakeSeam({ failRun: (cmd) => cmd[0] === "gh" }); - const ageKeySeam = new FakeAgeKeySeam(); - const { exitCode } = await runHomeInit(fakeProbes({}), seam, ageKeySeam); - - expect(exitCode).toBe(1); - // Only the gh check ran — filter-repo's check and every init step were - // never reached. - expect(seam.calls).toEqual([["gh", "auth", "status"]]); - expect(ageKeySeam.calls).toEqual([]); - }); - - test("preflight: a missing binary (spawn throws) is caught as an install hint, not a raw crash", async () => { - const seam = new FakeSeam({ throwOn: (cmd) => cmd[0] === "git" && cmd[1] === "filter-repo" }); - const ageKeySeam = new FakeAgeKeySeam(); - const { exitCode } = await runHomeInit(fakeProbes({}), seam, ageKeySeam); - - expect(exitCode).toBe(1); - expect(seam.calls).toEqual([ - ["gh", "auth", "status"], - ["git", "filter-repo", "--version"], - ]); - expect(ageKeySeam.calls).toEqual([]); - }); - - test("a fresh, fully successful init mints the age key and writes .sops.yaml as a distinct step after adoption, before returning", async () => { - const seam = new FakeSeam({ - failRun: (cmd) => cmd[0] === "gh" && cmd[1] === "repo" && cmd[2] === "view", // not-found -> falls through to create - stdout: (cmd) => - cmd[0] === "gh" && cmd[1] === "repo" && cmd[2] === "create" ? "https://github.com/testuser/mattstack-home\n" : "", - }); - const ageKeySeam = new FakeAgeKeySeam(); const sopsYamlSeam = new FakeSopsYamlSeam(); - // Minimal state -> no cruft, no user clone: createRepo, gitInit, - // writeGitignore, writeOwners, adoptCommit, push. const { exitCode, logs } = await runHomeInit(fakeProbes({}), seam, ageKeySeam, [], sopsYamlSeam); expect(exitCode).toBeUndefined(); - // The init steps ran to completion before the age key was touched. - expect(seam.calls.length).toBeGreaterThan(0); + const cloneCall = seam.calls.find((c) => c.kind === "run") as { kind: string; arg: string[] } | undefined; + expect(cloneCall?.arg).toEqual(["git", "clone", DEFAULT_USER_REPO_URL, "user"]); expect(ageKeySeam.calls.some((c) => c[1] === "find-generic-password")).toBe(true); expect(ageKeySeam.calls.some((c) => c[0] === "age-keygen")).toBe(true); - expect(ageKeySeam.calls.some((c) => c[1] === "add-generic-password")).toBe(true); expect(sopsYamlSeam.files.get(SOPS_YAML_PATH)).toBe(renderSopsYaml(FAKE_PUBLIC_KEY)); // The mint (and the age-key-ready line) happen BEFORE the success line — // never print success ahead of a mint that could still fail. const readyIdx = logs.findIndex((l) => l.includes("age key ready")); - const successIdx = logs.findIndex((l) => l.includes("is now the git-backed home repo")); + const successIdx = logs.findIndex((l) => l.includes("is provisioned")); expect(readyIdx).toBeGreaterThanOrEqual(0); expect(successIdx).toBeGreaterThan(readyIdx); }); - test("--dry-run never touches the age key, even on a fresh (not-yet-initialized) home", async () => { + test("--url overrides the default clone URL", async () => { + const seam = new FakeSeam(); + const customUrl = "https://github.com/example/mattstack-home.git"; + await runHomeInit(fakeProbes({}), seam, new FakeAgeKeySeam(), ["--url", customUrl]); + + const cloneCall = seam.calls.find((c) => c.kind === "run") as { kind: string; arg: string[] } | undefined; + expect(cloneCall?.arg).toEqual(["git", "clone", customUrl, "user"]); + }); + + test("--dry-run never touches the age key or runs any step, even on a fresh (not-yet-provisioned) home", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); const { exitCode } = await runHomeInit(fakeProbes({}), seam, ageKeySeam, ["--dry-run"]); @@ -297,11 +261,43 @@ describe("homeInit", () => { }); test("a failing init step aborts before the age key is ever touched", async () => { - const seam = new FakeSeam({ failRun: (cmd) => cmd.join(" ") === "git commit -m home: adopt the declarative layer" }); + const seam = new FakeSeam({ failRun: (cmd) => cmd[0] === "git" }); const ageKeySeam = new FakeAgeKeySeam(); const { exitCode } = await runHomeInit(fakeProbes({}), seam, ageKeySeam); expect(exitCode).toBe(1); expect(ageKeySeam.calls).toEqual([]); }); + + test("a real file at the skills.jsonc root path: still runs every other step, still mints the age key, but exits 1", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + const probes = fakeProbes({ + readSymlinkTarget: () => null, + exists: (path) => path.endsWith("skills.jsonc"), + }); + + const { exitCode, errors } = await runHomeInit(probes, seam, ageKeySeam); + + expect(exitCode).toBe(1); + expect(errors.some((e) => e.includes("refusing to overwrite"))).toBe(true); + expect(ageKeySeam.calls.some((c) => c[1] === "find-generic-password")).toBe(true); + expect(seam.calls.some((c) => c.kind === "writeSymlink")).toBe(false); + }); + + test("a real file at the skills.jsonc root path, --dry-run: reports the block and exits cleanly without running anything", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + const probes = fakeProbes({ + readSymlinkTarget: () => null, + exists: (path) => path.endsWith("skills.jsonc"), + }); + + const { exitCode, errors } = await runHomeInit(probes, seam, ageKeySeam, ["--dry-run"]); + + expect(exitCode).toBeUndefined(); + expect(errors.some((e) => e.includes("refusing to overwrite"))).toBe(true); + expect(seam.calls).toEqual([]); + expect(ageKeySeam.calls).toEqual([]); + }); }); diff --git a/commands/home.ts b/commands/home.ts index c641374b..e90f3c4e 100644 --- a/commands/home.ts +++ b/commands/home.ts @@ -1,21 +1,21 @@ /** - * rt home — the git-backed ~/.mattstack home repo. + * rt home — the git-backed ~/.mattstack/user personal repo, plus per-machine + * provisioning of the ~/.mattstack tree around it. * - * rt home init [--dry-run] print, then run, the adoption plan - * rt home key export print the age private key once, for a password manager + * rt home init [--dry-run] [--url ] print, then run, the provisioning plan + * rt home key export print the age private key once, for a password manager * * `init` gathers state, prints the plan from lib/home/init-plan.ts, and * (unless --dry-run) runs it through lib/home/init-exec.ts's injected seam. * `key export` delegates entirely to lib/home/age-key.ts. */ -import { existsSync, readFileSync, readdirSync, writeFileSync } from "fs"; +import { existsSync, readFileSync, readlinkSync, writeFileSync } from "fs"; import { join } from "path"; import type { CommandContext } from "../lib/command-tree.ts"; -import { mattstackHome, teamsDir } from "../lib/rt-paths.ts"; -import { buildInitPlan, type HomeState, type InitStep } from "../lib/home/init-plan.ts"; -import { createRealExecSeam, executeInitPlan, type ExecResult, type ExecSeam } from "../lib/home/init-exec.ts"; -import { parseOriginUrl } from "../lib/home/git-config.ts"; +import { machineKey, mattstackHome } from "../lib/rt-paths.ts"; +import { buildInitPlan, STATE_DIR_NAMES, type HomeState, type InitStep } from "../lib/home/init-plan.ts"; +import { createRealExecSeam, executeInitPlan, type ExecSeam } from "../lib/home/init-exec.ts"; import { AgeKeyAbsentError, createRealAgeKeySeam, @@ -26,15 +26,13 @@ import { type AgeKeySeam, } from "../lib/home/age-key.ts"; -/** Stray root cruft deleted at init time, not adopted into the repo. */ -const CRUFT_CANDIDATES = ["skills.jsonc.pre-pack", "skills.jsonc.retired-backup"]; +export const DEFAULT_USER_REPO_URL = "https://github.com/m4ttheweric/mattstack-home"; export interface HomeProbes { isGitRepo(dir: string): boolean; exists(path: string): boolean; - listTeamClones(): string[]; - /** Pure fs read; null when the file is missing or unreadable. */ - readFile(path: string): string | null; + /** The symlink's target, or null when `path` is absent or not a symlink. */ + readSymlinkTarget(path: string): string | null; } export interface SopsYamlSeam { @@ -59,20 +57,9 @@ function defaultProbes(): HomeProbes { return { isGitRepo: (dir) => existsSync(join(dir, ".git")), exists: (path) => existsSync(path), - listTeamClones: () => { - const dir = teamsDir(); - if (!existsSync(dir)) return []; + readSymlinkTarget: (path) => { try { - return readdirSync(dir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory() || entry.isSymbolicLink()) - .map((entry) => entry.name); - } catch { - return []; - } - }, - readFile: (path) => { - try { - return readFileSync(path, "utf8"); + return readlinkSync(path); } catch { return null; } @@ -80,61 +67,71 @@ function defaultProbes(): HomeProbes { }; } -export function gatherHomeState(home: string, probes: HomeProbes): HomeState { - // hasUserClone gates foldInPrefs, which runs `git filter-repo` against - // this directory — a plain (non-git) user/ must not trigger it. - const hasUserClone = probes.isGitRepo(join(home, "user")); - // Read while user/.git still exists — unlinkUserClone (lib/home/init-exec.ts) - // removes it before the fold-in re-clones from this URL. - const prefsRemoteUrl = hasUserClone - ? (parseOriginUrl(probes.readFile(join(home, "user", ".git", "config")) ?? "") ?? undefined) - : undefined; +const SKILLS_SYMLINK_TARGET = join("user", "skills.jsonc"); + +export function gatherHomeState(home: string, probes: HomeProbes, machineKeyValue: string): HomeState { + const userRepoPresent = probes.isGitRepo(join(home, "user")); + const machineKeyFilePresent = probes.exists(join(home, "machine-key")); + const profileDirPresent = probes.exists(join(home, "user", "local", machineKeyValue)); + + const skillsPath = join(home, "skills.jsonc"); + const symlinkTarget = probes.readSymlinkTarget(skillsPath); + const skillsSymlinkPresent = symlinkTarget === SKILLS_SYMLINK_TARGET; + const skillsSymlinkBlocked = symlinkTarget === null && probes.exists(skillsPath); + + const stateDirsMissing = STATE_DIR_NAMES.filter((name) => !probes.exists(join(home, name))); return { - isRepo: probes.isGitRepo(home), - hasUserClone, - hasTeamClones: probes.listTeamClones(), - cruft: CRUFT_CANDIDATES.filter((name) => probes.exists(join(home, name))), - prefsRemoteUrl, + userRepoPresent, + machineKeyFilePresent, + profileDirPresent, + skillsSymlinkPresent, + skillsSymlinkBlocked, + stateDirsMissing, }; } function describeStep(step: InitStep): string { switch (step.kind) { - case "createRepo": - return `create the private GitHub repo ${step.name}`; - case "gitInit": - return `git init -b ${step.branch}`; + case "ensureStateDirs": + return `create missing state dirs: ${step.dirs.join(", ")}`; + case "cloneUserRepo": + return `clone ${step.url} into user/`; case "writeGitignore": - return "write the boundary .gitignore"; + return "write the user repo's .gitignore"; case "writeOwners": - return "write snapshot-owners.jsonc"; - case "deleteCruft": - return `delete stray cruft: ${step.paths.join(", ")}`; - case "unlinkUserClone": - return "unlink user/.git (fold-in re-clones from the origin remote)"; - case "foldInPrefs": - return `fold mattstack-prefs history into user/ (git filter-repo, from ${step.sourceUrl})`; - case "adoptCommit": - return `commit: "${step.message}"`; - case "push": - return `push -u origin ${step.branch}`; + return "write user/snapshot-owners.jsonc"; + case "writeMachineKey": + return `write the machine-key file (${step.key})`; + case "ensureProfileDir": + return `create user/local/${step.key}/`; + case "writeSkillsSymlink": + return "link skills.jsonc -> user/skills.jsonc"; } } +function parseUrlArg(args: string[]): string { + const idx = args.indexOf("--url"); + const value = idx !== -1 ? args[idx + 1] : undefined; + return value && value.length > 0 ? value : DEFAULT_USER_REPO_URL; +} + /** * The sole mint site: `key export` (lib/home/age-key.ts:keyExport) refuses * to mint, precisely so a keychain-access error there can never be mistaken * for "no key yet". Idempotent (ensureAgeKey mints only on provable - * absence), so it's safe to run on every init — including the - * already-initialized short-circuit, for a home repo that predates this - * step. + * absence), so it's safe to run on every init — including a fully- + * provisioned machine, for a home repo that predates this step. * * Also (re)writes `.sops.yaml` whenever it's missing or its recipient * doesn't match the current key — the one place `rt secrets set` gets a * creation rule to encrypt against. A hand-edited file already carrying the * right recipient is left untouched. `.sops.yaml` is a TRACKED file, so a * write here needs a human commit — the snapshot daemon doesn't exist yet. + * + * Called only after the init plan (which clones user/ when it's missing) + * has run to completion, so user/ always already exists by the time this + * writes into it. */ async function ensureHomeAgeKey(seams: AgeKeySeam, sopsYamlSeam: SopsYamlSeam = defaultSopsYamlSeam()): Promise { const { publicKey } = await ensureAgeKey(seams); @@ -158,35 +155,6 @@ async function ensureHomeAgeKey(seams: AgeKeySeam, sopsYamlSeam: SopsYamlSeam = ); } -const GH_AUTH_HINT = "gh is not authenticated. Run:\n gh auth login"; -const FILTER_REPO_HINT = "git-filter-repo is not installed. Run:\n brew install git-filter-repo"; - -/** - * A missing binary makes the seam's `run()` throw (Bun.spawn rejects on - * ENOENT) rather than return a non-zero code, so each check needs its own - * catch — an uncaught throw here would surface as a raw stack instead of the - * install hint. - */ -async function preflight(exec: ExecSeam): Promise { - let auth: ExecResult; - try { - auth = await exec.run(["gh", "auth", "status"]); - } catch { - return GH_AUTH_HINT; - } - if (auth.code !== 0) return GH_AUTH_HINT; - - let filterRepo: ExecResult; - try { - filterRepo = await exec.run(["git", "filter-repo", "--version"]); - } catch { - return FILTER_REPO_HINT; - } - if (filterRepo.code !== 0) return FILTER_REPO_HINT; - - return null; -} - export async function homeInit( args: string[], _ctx: CommandContext = {}, @@ -194,38 +162,33 @@ export async function homeInit( exec: ExecSeam = createRealExecSeam(mattstackHome()), ageKeySeam: AgeKeySeam = createRealAgeKeySeam(), sopsYamlSeam: SopsYamlSeam = defaultSopsYamlSeam(), + // Evaluated at call time, like every other default here — a real fs read + // (~/.mattstack/machine-key), so tests inject a fixed value instead of + // depending on the test-runner's actual hostname/override file. + key: string = machineKey(), ): Promise { const dryRun = args.includes("--dry-run"); const home = mattstackHome(); - const state = gatherHomeState(home, probes); - const plan = buildInitPlan(state); - - if (plan.reason === "already-initialized") { - console.log(`rt home init: ${home} is already a git repo — nothing to do.`); - if (!dryRun) await ensureHomeAgeKey(ageKeySeam, sopsYamlSeam); - return; + const url = parseUrlArg(args); + const state = gatherHomeState(home, probes, key); + const plan = buildInitPlan(state, { url, machineKey: key }); + + if (plan.steps.length === 0) { + console.log(`rt home init: ${home} is already fully provisioned — nothing to do.`); + } else { + console.log(`rt home init plan for ${home}:`); + plan.steps.forEach((step, i) => console.log(` ${i + 1}. ${describeStep(step)}`)); } - if (plan.reason === "prefs-remote-unreadable") { + if (plan.blocked === "skills-symlink-real-file") { console.error( - `rt home init: could not read the origin URL from ${join(home, "user", ".git", "config")} — ` + - "refusing to fold in a remote it can't identify.", + `\nrt home init: a real file already exists at ${join(home, "skills.jsonc")} — refusing to overwrite it. ` + + "Move it aside by hand, then rerun.", ); - process.exit(1); } - console.log(`rt home init plan for ${home}:`); - plan.steps.forEach((step, i) => console.log(` ${i + 1}. ${describeStep(step)}`)); - if (dryRun) return; - const preflightError = await preflight(exec); - if (preflightError) { - console.error(`\nrt home init: preflight failed — nothing was run.\n${preflightError}`); - process.exit(1); - } - - console.log(""); const result = await executeInitPlan(plan.steps, exec, (message) => console.log(` ${message}`)); if (!result.ok) { @@ -237,7 +200,13 @@ export async function homeInit( // failed mint would tell the operator init worked while `rt secrets set` // still has no key or creation rule to encrypt against. await ensureHomeAgeKey(ageKeySeam, sopsYamlSeam); - console.log(`\nrt home init: ${home} is now the git-backed home repo.`); + + if (plan.blocked === "skills-symlink-real-file") { + console.error(`\nrt home init: provisioning finished, but the skills.jsonc symlink is still blocked — see above.`); + process.exit(1); + } + + console.log(`\nrt home init: ${home} is provisioned.`); } export async function homeKeyExport( diff --git a/lib/command-tree-def.ts b/lib/command-tree-def.ts index 1c3a23aa..22674020 100644 --- a/lib/command-tree-def.ts +++ b/lib/command-tree-def.ts @@ -692,14 +692,20 @@ export const TREE: Record = { }, home: { - description: "The git-backed ~/.mattstack home repo", + description: "The git-backed ~/.mattstack/user personal repo", subcommands: { init: { - description: "Provision the home repo: print, then run, the adoption plan", + description: "Provision this machine: clone the user repo, then print and run the provisioning plan", module: "./commands/home.ts", fn: "homeInit", args: [ { name: "Dry run", flag: "--dry-run", type: "boolean", default: false, hint: "Print the plan without running it" }, + { + name: "Clone URL", + flag: "--url", + type: "text", + hint: "The user repo to clone (default: https://github.com/m4ttheweric/mattstack-home)", + }, ], }, key: { diff --git a/lib/home/__tests__/boundary.test.ts b/lib/home/__tests__/boundary.test.ts index 1428f934..61489695 100644 --- a/lib/home/__tests__/boundary.test.ts +++ b/lib/home/__tests__/boundary.test.ts @@ -1,122 +1,21 @@ import { describe, test, expect } from "bun:test"; import { HOME_BOUNDARY, renderHomeGitignore } from "../boundary.ts"; -/** - * Minimal gitignore-semantics matcher for the patterns this module emits. - * Models the one distinction that matters here: a pattern containing a "/" - * anywhere but the very end (a leading slash, or a slash in the middle) is - * anchored to the root and only matches there; a pattern with no such slash - * matches its directory/file name at ANY depth. Not a general gitignore - * engine — just enough to prove renderHomeGitignore() draws the boundary at - * the depth the spec describes. - */ -function isAnchored(pattern: string): boolean { - const withoutTrailingSlash = pattern.endsWith("/") ? pattern.slice(0, -1) : pattern; - return withoutTrailingSlash.includes("/"); -} - -function gitignorePatterns(gitignore: string): string[] { - return gitignore - .split("\n") - .map((line) => line.trim()) - .filter((line) => line.length > 0 && !line.startsWith("#")); -} - -/** `*` never crosses a `/` — enough glob support for this module's own patterns (`*.sock`, `user/secrets/*.tmp`). */ -function globToRegExp(body: string): RegExp { - const escaped = body.replace(/[.+^${}()|[\]\\]/g, "\\$&").replace(/\*/g, "[^/]*"); - return new RegExp(`^${escaped}$`); -} - -function isIgnored(patterns: string[], path: string): boolean { - const segments = path.split("/"); - const base = segments[segments.length - 1]!; - const dirSegments = segments.slice(0, -1); - - return patterns.some((pattern) => { - const anchored = isAnchored(pattern); - const body = pattern.startsWith("/") ? pattern.slice(1) : pattern; - - if (body.endsWith("/")) { - const dir = body.slice(0, -1); - return anchored ? path === dir || path.startsWith(`${dir}/`) : dirSegments.includes(dir); - } - const regex = globToRegExp(body); - return anchored ? regex.test(path) : regex.test(base); - }); -} - describe("HOME_BOUNDARY", () => { - test("declares exactly the ruled ignore set, root-anchored for single-segment dirs", () => { - expect(HOME_BOUNDARY.ignored).toEqual([ - "/rt/", - "/deck/", - "/shepherdr/", - "/repos/", - "/ci-attendants/", - "/work/", - "/teams/", - "user/local/", - "settings.local.jsonc", - "user/secrets/*.tmp", - "*.sock", - ".DS_Store", - ]); - }); - - test("declares the tracked declarative surfaces", () => { - expect(HOME_BOUNDARY.tracked).toEqual([ - "user/", - "skills.jsonc", - "snapshot-owners.jsonc", - "user/secrets/", - ]); + test("declares exactly the ruled hygiene set — no local/ line", () => { + expect(HOME_BOUNDARY.ignored).toEqual([".DS_Store", "*.sock", "*.tmp"]); }); }); describe("renderHomeGitignore", () => { - const patterns = gitignorePatterns(renderHomeGitignore()); - - const ignoredCases = [ - "rt/state.db", - "rt/rt.sock", - "deck/settings.json", - "shepherdr/jobs/1.json", - "repos/assured-dev/config.json", - "ci-attendants/foo", - "work/scratch", - "teams/claimview/mattstack/settings.jsonc", - "user/local/attic.tar", - "settings.local.jsonc", - ".DS_Store", - "user/.DS_Store", - "deck-api.sock", - "user/secrets/rt.json.tmp", - ]; - - const trackedCases = [ - "user/", - "user/settings.jsonc", - "skills.jsonc", - "snapshot-owners.jsonc", - "user/secrets/", - "user/secrets/rt.json", - ]; - - for (const path of ignoredCases) { - test(`ignores ${path}`, () => { - expect(isIgnored(patterns, path)).toBe(true); - }); - } - - for (const path of trackedCases) { - test(`does not ignore ${path}`, () => { - expect(isIgnored(patterns, path)).toBe(false); - }); - } + test("renders one pattern per line, trailing newline", () => { + expect(renderHomeGitignore()).toBe(".DS_Store\n*.sock\n*.tmp\n"); + }); - test("root-anchored dir patterns do not match the same name at depth", () => { - expect(isIgnored(patterns, "rt/x")).toBe(true); - expect(isIgnored(patterns, "user/rt/x")).toBe(false); + test("does not ignore user/local/ — machine profiles must stay tracked", () => { + const lines = renderHomeGitignore() + .split("\n") + .filter((l) => l.length > 0); + expect(lines.some((l) => l.includes("local"))).toBe(false); }); }); diff --git a/lib/home/__tests__/init-exec.test.ts b/lib/home/__tests__/init-exec.test.ts index 2b593630..c2790500 100644 --- a/lib/home/__tests__/init-exec.test.ts +++ b/lib/home/__tests__/init-exec.test.ts @@ -1,387 +1,184 @@ import { describe, test, expect } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "fs"; +import { existsSync, lstatSync, mkdtempSync, readFileSync, readlinkSync, realpathSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { createRealExecSeam, executeInitPlan, type ExecResult, type ExecSeam } from "../init-exec.ts"; -import { buildInitPlan, type InitStep } from "../init-plan.ts"; +import { buildInitPlan, STATE_DIR_NAMES, type InitStep } from "../init-plan.ts"; -const PREFS_URL = "https://github.com/mattgoodwin/mattstack-prefs.git"; -const CREATED_URL = "https://github.com/testuser/mattstack-home"; +const REPO_URL = "https://github.com/m4ttheweric/mattstack-home"; type RecordedCall = | { kind: "run"; cmd: string[]; cwd?: string } | { kind: "writeFile"; path: string; content: string } - | { kind: "removeDir"; path: string } - | { kind: "mkTempDir" }; + | { kind: "mkdirp"; path: string } + | { kind: "writeSymlink"; path: string; target: string }; const noopLog = () => {}; -function isGhRepoView(cmd: string[]): boolean { - return cmd[0] === "gh" && cmd[1] === "repo" && cmd[2] === "view"; -} -function isGhRepoCreate(cmd: string[]): boolean { - return cmd[0] === "gh" && cmd[1] === "repo" && cmd[2] === "create"; -} - /** Records every seam call in order; never touches a real fs or subprocess. */ class FakeExecSeam implements ExecSeam { calls: RecordedCall[] = []; - constructor( - private opts: { - stdout?: (cmd: string[]) => string; - failRun?: (cmd: string[]) => string | undefined; - failWriteFile?: string; - } = {}, - ) {} + constructor(private opts: { failRun?: (cmd: string[]) => string | undefined } = {}) {} async run(cmd: string[], runOpts?: { cwd?: string }): Promise { this.calls.push({ kind: "run", cmd, cwd: runOpts?.cwd }); const failure = this.opts.failRun?.(cmd); if (failure) return { code: 1, stdout: "", stderr: failure }; - return { code: 0, stdout: this.opts.stdout?.(cmd) ?? "", stderr: "" }; + return { code: 0, stdout: "", stderr: "" }; } async writeFile(path: string, content: string): Promise { this.calls.push({ kind: "writeFile", path, content }); - if (this.opts.failWriteFile === path) throw new Error(`write failed: ${path}`); } - async removeDir(path: string): Promise { - this.calls.push({ kind: "removeDir", path }); + async mkdirp(path: string): Promise { + this.calls.push({ kind: "mkdirp", path }); } - async mkTempDir(): Promise { - this.calls.push({ kind: "mkTempDir" }); - return "/tmp/rt-home-fold-test"; + async writeSymlink(path: string, target: string): Promise { + this.calls.push({ kind: "writeSymlink", path, target }); } } -/** `gh repo view` reports "not found"; the step falls through to `gh repo create`. */ -function repoNotFoundThenCreated(url: string = CREATED_URL): ConstructorParameters[0] { - return { - failRun: (cmd) => (isGhRepoView(cmd) ? "GraphQL: Could not resolve to a Repository" : undefined), - stdout: (cmd) => (isGhRepoCreate(cmd) ? `${url}\n` : ""), - }; -} - describe("executeInitPlan", () => { - describe("createRepo (resume-safe)", () => { - test("gh repo view reports not-found: falls through to gh repo create", async () => { - const seam = new FakeExecSeam(repoNotFoundThenCreated()); - const steps: InitStep[] = [{ kind: "createRepo", name: "mattstack-home" }]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - { kind: "run", cmd: ["gh", "repo", "create", "mattstack-home", "--private"], cwd: undefined }, - ]); - }); - - test("gh repo view reports an existing EMPTY repo: reuses its url, never calls gh repo create", async () => { - const seam = new FakeExecSeam({ - stdout: (cmd) => (isGhRepoView(cmd) ? JSON.stringify({ isEmpty: true, url: CREATED_URL }) : ""), - }); - const steps: InitStep[] = [{ kind: "createRepo", name: "mattstack-home" }, { kind: "gitInit", branch: "main" }]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - { kind: "run", cmd: ["git", "init", "-b", "main"], cwd: undefined }, - { kind: "run", cmd: ["git", "remote", "add", "origin", CREATED_URL], cwd: undefined }, - ]); - }); - - test("gh repo view reports an existing NON-EMPTY repo: fails naming the conflict, never creates or inits", async () => { - const seam = new FakeExecSeam({ - stdout: (cmd) => (isGhRepoView(cmd) ? JSON.stringify({ isEmpty: false, url: CREATED_URL }) : ""), - }); - const steps: InitStep[] = [{ kind: "createRepo", name: "mattstack-home" }, { kind: "gitInit", branch: "main" }]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.failedStep).toBe("createRepo"); - expect(result.stderr).toContain("mattstack-home"); - expect(result.stderr).toContain("already exists"); - } - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - ]); - }); - - test("empty gh repo create stdout fails the step instead of silently skipping remote add", async () => { - const seam = new FakeExecSeam({ - failRun: (cmd) => (isGhRepoView(cmd) ? "not found" : undefined), - // no stdout scripted for create -> gh prints nothing - }); - const steps: InitStep[] = [{ kind: "createRepo", name: "mattstack-home" }, { kind: "gitInit", branch: "main" }]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.failedStep).toBe("createRepo"); - expect(result.stderr).toBe("gh repo create printed no repo URL"); - } - // gitInit never ran. - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - { kind: "run", cmd: ["gh", "repo", "create", "mattstack-home", "--private"], cwd: undefined }, - ]); - }); - }); - - test("gitInit: init -b , then wires origin to the URL gh printed", async () => { - const seam = new FakeExecSeam(repoNotFoundThenCreated()); - const steps: InitStep[] = [ - { kind: "createRepo", name: "mattstack-home" }, - { kind: "gitInit", branch: "main" }, - ]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - { kind: "run", cmd: ["gh", "repo", "create", "mattstack-home", "--private"], cwd: undefined }, - { kind: "run", cmd: ["git", "init", "-b", "main"], cwd: undefined }, - { kind: "run", cmd: ["git", "remote", "add", "origin", CREATED_URL], cwd: undefined }, - ]); - }); - - test("writeGitignore and writeOwners write the step's rendered content verbatim", async () => { + test("ensureStateDirs: mkdirp's each missing dir in order", async () => { const seam = new FakeExecSeam(); - const steps: InitStep[] = [ - { kind: "writeGitignore", content: "/rt/\n" }, - { kind: "writeOwners", content: "{}\n" }, - ]; + const steps: InitStep[] = [{ kind: "ensureStateDirs", dirs: ["rt", "work"] }]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); expect(seam.calls).toEqual([ - { kind: "writeFile", path: ".gitignore", content: "/rt/\n" }, - { kind: "writeFile", path: "snapshot-owners.jsonc", content: "{}\n" }, + { kind: "mkdirp", path: "rt" }, + { kind: "mkdirp", path: "work" }, ]); }); - test("deleteCruft removes each path via the seam, not shell rm", async () => { + test("cloneUserRepo: git clone user, cwd defaults to home", async () => { const seam = new FakeExecSeam(); - const steps: InitStep[] = [ - { kind: "deleteCruft", paths: ["skills.jsonc.pre-pack", "skills.jsonc.retired-backup"] }, - ]; + const steps: InitStep[] = [{ kind: "cloneUserRepo", url: REPO_URL }]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "removeDir", path: "skills.jsonc.pre-pack" }, - { kind: "removeDir", path: "skills.jsonc.retired-backup" }, - ]); + expect(seam.calls).toEqual([{ kind: "run", cmd: ["git", "clone", REPO_URL, "user"], cwd: undefined }]); }); - test("unlinkUserClone removes user/.git via the seam, not shell rm", async () => { - const seam = new FakeExecSeam(); - const steps: InitStep[] = [{ kind: "unlinkUserClone" }]; + test("cloneUserRepo: a failing clone aborts and reports it", async () => { + const seam = new FakeExecSeam({ failRun: () => "fatal: could not read from remote repository" }); + const steps: InitStep[] = [{ kind: "cloneUserRepo", url: REPO_URL }]; const result = await executeInitPlan(steps, seam, noopLog); - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([{ kind: "removeDir", path: "user/.git" }]); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.failedStep).toBe("cloneUserRepo"); + expect(result.stderr).toContain("could not read from remote"); + } }); - test("foldInPrefs: clones step.sourceUrl, filter-repo in the clone, fetch HEAD + merge in the home repo, then removes the temp clone", async () => { + test("writeGitignore and writeOwners write into the user repo, not the root", async () => { const seam = new FakeExecSeam(); - const steps: InitStep[] = [{ kind: "foldInPrefs", sourceUrl: PREFS_URL }]; + const steps: InitStep[] = [ + { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeOwners", content: "{}\n" }, + ]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); expect(seam.calls).toEqual([ - { kind: "mkTempDir" }, - { - kind: "run", - cmd: ["git", "clone", "--no-hardlinks", PREFS_URL, "/tmp/rt-home-fold-test"], - cwd: undefined, - }, - { - kind: "run", - cmd: ["git", "filter-repo", "--to-subdirectory-filter", "user"], - cwd: "/tmp/rt-home-fold-test", - }, - // HEAD, not a hardcoded branch name: the tmp clone's default branch - // IS whatever the source remote's default branch is. - { kind: "run", cmd: ["git", "fetch", "/tmp/rt-home-fold-test", "HEAD"], cwd: undefined }, - { - kind: "run", - cmd: [ - "git", - "merge", - "FETCH_HEAD", - "--allow-unrelated-histories", - "-m", - "home: fold in mattstack-prefs history under user/", - ], - cwd: undefined, - }, - { kind: "removeDir", path: "/tmp/rt-home-fold-test" }, + { kind: "writeFile", path: "user/.gitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeFile", path: "user/snapshot-owners.jsonc", content: "{}\n" }, ]); }); - test("foldInPrefs: the temp clone is removed even when a step inside it fails", async () => { - const seam = new FakeExecSeam({ - failRun: (cmd) => (cmd[1] === "filter-repo" ? "filter-repo: boom" : undefined), - }); - const steps: InitStep[] = [{ kind: "foldInPrefs", sourceUrl: PREFS_URL }]; + test("writeMachineKey: writes the key to the root machine-key file", async () => { + const seam = new FakeExecSeam(); + const steps: InitStep[] = [{ kind: "writeMachineKey", key: "mbp-14" }]; const result = await executeInitPlan(steps, seam, noopLog); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.failedStep).toBe("foldInPrefs"); - expect(result.stderr).toBe("filter-repo: boom"); - } - expect(seam.calls.at(-1)).toEqual({ kind: "removeDir", path: "/tmp/rt-home-fold-test" }); + expect(result).toEqual({ ok: true }); + expect(seam.calls).toEqual([{ kind: "writeFile", path: "machine-key", content: "mbp-14" }]); }); - test("adoptCommit: add -A then commit with the plan's message", async () => { + test("ensureProfileDir: mkdirp's user/local//", async () => { const seam = new FakeExecSeam(); - const steps: InitStep[] = [{ kind: "adoptCommit", message: "home: adopt the declarative layer" }]; + const steps: InitStep[] = [{ kind: "ensureProfileDir", key: "mbp-14" }]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["git", "add", "-A"], cwd: undefined }, - { kind: "run", cmd: ["git", "commit", "-m", "home: adopt the declarative layer"], cwd: undefined }, - ]); + expect(seam.calls).toEqual([{ kind: "mkdirp", path: join("user", "local", "mbp-14") }]); }); - test("push: -u origin ", async () => { + test("writeSkillsSymlink: links the root path to user/skills.jsonc", async () => { const seam = new FakeExecSeam(); - const steps: InitStep[] = [{ kind: "push", branch: "main" }]; + const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([{ kind: "run", cmd: ["git", "push", "-u", "origin", "main"], cwd: undefined }]); + expect(seam.calls).toEqual([{ kind: "writeSymlink", path: "skills.jsonc", target: join("user", "skills.jsonc") }]); }); - test("runs a full plan's steps in order", async () => { - const seam = new FakeExecSeam(repoNotFoundThenCreated()); - const steps: InitStep[] = [ - { kind: "createRepo", name: "mattstack-home" }, - { kind: "gitInit", branch: "main" }, - { kind: "writeGitignore", content: "/rt/\n" }, - { kind: "writeOwners", content: "{}\n" }, - { kind: "deleteCruft", paths: ["skills.jsonc.pre-pack"] }, - { kind: "unlinkUserClone" }, - { kind: "adoptCommit", message: "home: adopt the declarative layer" }, - { kind: "foldInPrefs", sourceUrl: PREFS_URL }, - { kind: "push", branch: "main" }, - ]; + test("runs a full fresh-machine plan's steps in order", async () => { + const seam = new FakeExecSeam(); + const steps = buildInitPlan( + { + userRepoPresent: false, + machineKeyFilePresent: false, + profileDirPresent: false, + skillsSymlinkPresent: false, + skillsSymlinkBlocked: false, + stateDirsMissing: [...STATE_DIR_NAMES], + }, + { url: REPO_URL, machineKey: "mbp-14" }, + ).steps; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); expect(seam.calls.map((c) => c.kind)).toEqual([ - "run", // gh repo view - "run", // gh repo create - "run", // git init - "run", // git remote add origin - "writeFile", // .gitignore - "writeFile", // snapshot-owners.jsonc - "removeDir", // deleteCruft - "removeDir", // unlinkUserClone: user/.git - "run", // git add -A - "run", // git commit - "mkTempDir", + "mkdirp", // rt + "mkdirp", // deck + "mkdirp", // shepherdr + "mkdirp", // repos + "mkdirp", // work + "mkdirp", // teams "run", // git clone - "run", // git filter-repo - "run", // git fetch - "run", // git merge - "removeDir", // temp clone cleanup - "run", // git push + "writeFile", // user/.gitignore + "writeFile", // user/snapshot-owners.jsonc + "writeFile", // machine-key + "mkdirp", // user/local/mbp-14 + "writeSymlink", // skills.jsonc ]); }); - test("gitlink regression: unlinkUserClone's removeDir(user/.git) runs before adoptCommit's git add -A", async () => { - const seam = new FakeExecSeam(repoNotFoundThenCreated()); - const steps = buildInitPlan({ - isRepo: false, - hasUserClone: true, - hasTeamClones: [], - cruft: [], - prefsRemoteUrl: PREFS_URL, - }).steps; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - const unlinkIndex = seam.calls.findIndex((c) => c.kind === "removeDir" && c.path === "user/.git"); - const addIndex = seam.calls.findIndex((c) => c.kind === "run" && c.cmd.join(" ") === "git add -A"); - expect(unlinkIndex).toBeGreaterThanOrEqual(0); - expect(addIndex).toBeGreaterThan(unlinkIndex); - }); - test("a failing step aborts the remaining steps and reports it", async () => { - const seam = new FakeExecSeam({ - failRun: (cmd) => (isGhRepoView(cmd) ? "not found" : undefined), - failWriteFile: ".gitignore", - stdout: (cmd) => (isGhRepoCreate(cmd) ? `${CREATED_URL}\n` : ""), - }); + const seam = new FakeExecSeam({ failRun: (cmd) => (cmd[0] === "git" ? "fatal: repository not found" : undefined) }); const steps: InitStep[] = [ - { kind: "createRepo", name: "mattstack-home" }, - { kind: "gitInit", branch: "main" }, - { kind: "writeGitignore", content: "/rt/\n" }, - { kind: "writeOwners", content: "{}\n" }, - { kind: "adoptCommit", message: "home: adopt the declarative layer" }, - { kind: "push", branch: "main" }, + { kind: "cloneUserRepo", url: REPO_URL }, + { kind: "writeGitignore", content: ".DS_Store\n" }, + { kind: "writeMachineKey", key: "mbp-14" }, ]; const result = await executeInitPlan(steps, seam, noopLog); expect(result.ok).toBe(false); if (!result.ok) { - expect(result.failedStep).toBe("writeGitignore"); - expect(result.stderr).toContain(".gitignore"); + expect(result.failedStep).toBe("cloneUserRepo"); + expect(result.stderr).toContain("not found"); } - // writeOwners, adoptCommit and push never ran. - expect(seam.calls.map((c) => c.kind)).toEqual(["run", "run", "run", "run", "writeFile"]); - }); - - test("a failing subprocess (non-zero exit) also aborts the remainder", async () => { - const seam = new FakeExecSeam({ - failRun: (cmd) => (cmd[0] === "gh" ? "gh: not authenticated" : undefined), - }); - const steps: InitStep[] = [ - { kind: "createRepo", name: "mattstack-home" }, - { kind: "gitInit", branch: "main" }, - ]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.failedStep).toBe("createRepo"); - expect(result.stderr).toBe("gh: not authenticated"); - } - // gh repo view fails (treated as not-found) then gh repo create also fails. - expect(seam.calls).toEqual([ - { kind: "run", cmd: ["gh", "repo", "view", "mattstack-home", "--json", "isEmpty,url"], cwd: undefined }, - { kind: "run", cmd: ["gh", "repo", "create", "mattstack-home", "--private"], cwd: undefined }, - ]); + // writeGitignore and writeMachineKey never ran. + expect(seam.calls).toEqual([{ kind: "run", cmd: ["git", "clone", REPO_URL, "user"], cwd: undefined }]); }); }); describe("createRealExecSeam", () => { - test("run() defaults cwd to home, writeFile/removeDir resolve relative paths against home, absolute paths pass through", async () => { + test("run() defaults cwd to home; mkdirp/writeFile/writeSymlink resolve relative to home", async () => { const home = mkdtempSync(join(tmpdir(), "rt-home-exec-test-")); try { const seam = createRealExecSeam(home); @@ -390,22 +187,20 @@ describe("createRealExecSeam", () => { expect(pwd.code).toBe(0); expect(realpathSync(pwd.stdout.trim())).toBe(realpathSync(home)); - await seam.writeFile("hello.txt", "hi\n"); - expect(readFileSync(join(home, "hello.txt"), "utf8")).toBe("hi\n"); - - await seam.removeDir("hello.txt"); - expect(existsSync(join(home, "hello.txt"))).toBe(false); + await seam.mkdirp(join("user", "local", "mbp-14")); + expect(existsSync(join(home, "user", "local", "mbp-14"))).toBe(true); - const tmp = await seam.mkTempDir(); - expect(existsSync(tmp)).toBe(true); - expect(tmp.startsWith(home)).toBe(false); + await seam.writeFile("user/skills.jsonc", "{}\n"); + expect(readFileSync(join(home, "user", "skills.jsonc"), "utf8")).toBe("{}\n"); - const outsideFile = join(tmpdir(), `rt-home-exec-outside-${Date.now()}`); - writeFileSync(outsideFile, "x"); - await seam.removeDir(outsideFile); - expect(existsSync(outsideFile)).toBe(false); + await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); + const st = lstatSync(join(home, "skills.jsonc")); + expect(st.isSymbolicLink()).toBe(true); + expect(readlinkSync(join(home, "skills.jsonc"))).toBe(join("user", "skills.jsonc")); - await seam.removeDir(tmp); + // writeSymlink replaces whatever was already there. + await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); + expect(lstatSync(join(home, "skills.jsonc")).isSymbolicLink()).toBe(true); } finally { rmSync(home, { recursive: true, force: true }); } diff --git a/lib/home/__tests__/init-plan.test.ts b/lib/home/__tests__/init-plan.test.ts index 7b980240..32e29b44 100644 --- a/lib/home/__tests__/init-plan.test.ts +++ b/lib/home/__tests__/init-plan.test.ts @@ -1,118 +1,137 @@ import { describe, test, expect } from "bun:test"; -import { buildInitPlan, type HomeState } from "../init-plan.ts"; - -const PREFS_URL = "https://github.com/mattgoodwin/mattstack-prefs.git"; +import { buildInitPlan, STATE_DIR_NAMES, type HomeState, type InitPlanConfig } from "../init-plan.ts"; + +const CONFIG: InitPlanConfig = { url: "https://github.com/m4ttheweric/mattstack-home", machineKey: "mbp-14" }; + +const FRESH_STATE: HomeState = { + userRepoPresent: false, + machineKeyFilePresent: false, + profileDirPresent: false, + skillsSymlinkPresent: false, + skillsSymlinkBlocked: false, + stateDirsMissing: [...STATE_DIR_NAMES], +}; + +const FULLY_PROVISIONED_STATE: HomeState = { + userRepoPresent: true, + machineKeyFilePresent: true, + profileDirPresent: true, + skillsSymlinkPresent: true, + skillsSymlinkBlocked: false, + stateDirsMissing: [], +}; describe("buildInitPlan", () => { - test("orders a fresh-adoption plan createRepo through push, unlink before adopt, adopt before fold", () => { - const state: HomeState = { - isRepo: false, - hasUserClone: true, - hasTeamClones: ["claimview"], - cruft: ["skills.jsonc.pre-pack", "skills.jsonc.retired-backup"], - prefsRemoteUrl: PREFS_URL, - }; + test("fresh HOME: emits every step in order", () => { + const plan = buildInitPlan(FRESH_STATE, CONFIG); - const plan = buildInitPlan(state); - - expect(plan.reason).toBeUndefined(); + expect(plan.blocked).toBeUndefined(); expect(plan.steps.map((s) => s.kind)).toEqual([ - "createRepo", - "gitInit", + "ensureStateDirs", + "cloneUserRepo", "writeGitignore", "writeOwners", - "deleteCruft", - "unlinkUserClone", - "adoptCommit", - "foldInPrefs", - "push", + "writeMachineKey", + "ensureProfileDir", + "writeSkillsSymlink", ]); - - const foldInPrefs = plan.steps.find((s) => s.kind === "foldInPrefs"); - expect(foldInPrefs).toEqual({ kind: "foldInPrefs", sourceUrl: PREFS_URL }); + expect(plan.steps[0]).toEqual({ kind: "ensureStateDirs", dirs: STATE_DIR_NAMES }); + expect(plan.steps[1]).toEqual({ kind: "cloneUserRepo", url: CONFIG.url }); + expect(plan.steps.find((s) => s.kind === "writeMachineKey")).toEqual({ + kind: "writeMachineKey", + key: "mbp-14", + }); + expect(plan.steps.find((s) => s.kind === "ensureProfileDir")).toEqual({ + kind: "ensureProfileDir", + key: "mbp-14", + }); }); - test("carries the cruft paths onto the deleteCruft step", () => { + test("repo present: skips clone and the gitignore/owners that ride with it — provisioning-only", () => { const state: HomeState = { - isRepo: false, - hasUserClone: true, - hasTeamClones: [], - cruft: ["skills.jsonc.pre-pack", "skills.jsonc.retired-backup"], - prefsRemoteUrl: PREFS_URL, + ...FRESH_STATE, + userRepoPresent: true, + stateDirsMissing: [], }; - const plan = buildInitPlan(state); - const deleteCruft = plan.steps.find((s) => s.kind === "deleteCruft"); - expect(deleteCruft).toEqual({ - kind: "deleteCruft", - paths: ["skills.jsonc.pre-pack", "skills.jsonc.retired-backup"], - }); - }); + const plan = buildInitPlan(state, CONFIG); - test("omits foldInPrefs and unlinkUserClone when there is no user clone to adopt", () => { - const state: HomeState = { isRepo: false, hasUserClone: false, hasTeamClones: [], cruft: [] }; - const plan = buildInitPlan(state); - expect(plan.steps.map((s) => s.kind)).not.toContain("foldInPrefs"); - expect(plan.steps.map((s) => s.kind)).not.toContain("unlinkUserClone"); + expect(plan.steps.map((s) => s.kind)).toEqual(["writeMachineKey", "ensureProfileDir", "writeSkillsSymlink"]); }); - test("omits deleteCruft when there is no stray cruft", () => { - const state: HomeState = { isRepo: false, hasUserClone: false, hasTeamClones: [], cruft: [] }; - const plan = buildInitPlan(state); - expect(plan.steps.map((s) => s.kind)).not.toContain("deleteCruft"); - }); + test("real file at the symlink path: blocked, but every other applicable step still runs", () => { + const state: HomeState = { + ...FRESH_STATE, + skillsSymlinkBlocked: true, + }; - test("push carries the same branch gitInit created", () => { - const state: HomeState = { isRepo: false, hasUserClone: false, hasTeamClones: [], cruft: [] }; - const plan = buildInitPlan(state); - const gitInit = plan.steps.find((s) => s.kind === "gitInit"); - const push = plan.steps.find((s) => s.kind === "push"); - expect(gitInit).toBeDefined(); - expect(push).toBeDefined(); - expect((push as { branch: string }).branch).toBe((gitInit as { branch: string }).branch); + const plan = buildInitPlan(state, CONFIG); + + expect(plan.blocked).toBe("skills-symlink-real-file"); + expect(plan.steps.map((s) => s.kind)).not.toContain("writeSkillsSymlink"); + expect(plan.steps.map((s) => s.kind)).toEqual([ + "ensureStateDirs", + "cloneUserRepo", + "writeGitignore", + "writeOwners", + "writeMachineKey", + "ensureProfileDir", + ]); }); - test("already-initialized: returns no steps plus the reason", () => { - const state: HomeState = { - isRepo: true, - hasUserClone: true, - hasTeamClones: ["claimview"], - cruft: ["skills.jsonc.pre-pack"], - prefsRemoteUrl: PREFS_URL, - }; + test("machine-key file present: no writeMachineKey step, but profile dir and symlink are unaffected", () => { + const state: HomeState = { ...FRESH_STATE, machineKeyFilePresent: true }; - const plan = buildInitPlan(state); + const plan = buildInitPlan(state, CONFIG); - expect(plan.steps).toEqual([]); - expect(plan.reason).toBe("already-initialized"); + expect(plan.steps.map((s) => s.kind)).not.toContain("writeMachineKey"); + expect(plan.steps.map((s) => s.kind)).toEqual([ + "ensureStateDirs", + "cloneUserRepo", + "writeGitignore", + "writeOwners", + "ensureProfileDir", + "writeSkillsSymlink", + ]); }); - test("prefs-remote-unreadable: a user clone with no parseable origin URL fails loudly instead of emitting an unrunnable fold", () => { - const state: HomeState = { - isRepo: false, - hasUserClone: true, - hasTeamClones: [], - cruft: [], - prefsRemoteUrl: undefined, - }; + test("profile dir present: no ensureProfileDir step", () => { + const state: HomeState = { ...FRESH_STATE, profileDirPresent: true }; + const plan = buildInitPlan(state, CONFIG); + expect(plan.steps.map((s) => s.kind)).not.toContain("ensureProfileDir"); + }); - const plan = buildInitPlan(state); + test("skills symlink already correct: no writeSkillsSymlink step", () => { + const state: HomeState = { ...FRESH_STATE, skillsSymlinkPresent: true }; + const plan = buildInitPlan(state, CONFIG); + expect(plan.steps.map((s) => s.kind)).not.toContain("writeSkillsSymlink"); + }); - expect(plan.steps).toEqual([]); - expect(plan.reason).toBe("prefs-remote-unreadable"); + test("no state dirs missing: no ensureStateDirs step", () => { + const state: HomeState = { ...FRESH_STATE, stateDirsMissing: [] }; + const plan = buildInitPlan(state, CONFIG); + expect(plan.steps.map((s) => s.kind)).not.toContain("ensureStateDirs"); }); - test("isRepo takes precedence over an unreadable prefs remote", () => { - const state: HomeState = { - isRepo: true, - hasUserClone: true, - hasTeamClones: [], - cruft: [], - prefsRemoteUrl: undefined, - }; + test("ensureStateDirs carries only the missing dirs, not the full list", () => { + const state: HomeState = { ...FRESH_STATE, stateDirsMissing: ["work", "teams"] }; + const plan = buildInitPlan(state, CONFIG); + expect(plan.steps.find((s) => s.kind === "ensureStateDirs")).toEqual({ + kind: "ensureStateDirs", + dirs: ["work", "teams"], + }); + }); - const plan = buildInitPlan(state); + test("fully provisioned: empty plan, not blocked", () => { + const plan = buildInitPlan(FULLY_PROVISIONED_STATE, CONFIG); + expect(plan.steps).toEqual([]); + expect(plan.blocked).toBeUndefined(); + }); - expect(plan.reason).toBe("already-initialized"); + test("blocked takes precedence even when nothing else needs doing", () => { + const state: HomeState = { ...FULLY_PROVISIONED_STATE, skillsSymlinkPresent: false, skillsSymlinkBlocked: true }; + const plan = buildInitPlan(state, CONFIG); + expect(plan.steps).toEqual([]); + expect(plan.blocked).toBe("skills-symlink-real-file"); }); }); diff --git a/lib/home/boundary.ts b/lib/home/boundary.ts index 57a6c25e..437ed4de 100644 --- a/lib/home/boundary.ts +++ b/lib/home/boundary.ts @@ -1,34 +1,15 @@ /** - * The home repo's gitignore boundary. + * The user repo's gitignore hygiene list. * - * The gitignore IS the boundary: tracked = declarative surfaces (synced - * across machines), ignored = runtime. `user/local/` is hoisted here rather - * than left to mattstack-prefs' own folded-in `.gitignore`, so the boundary - * doesn't depend on that inner file surviving the fold-in. - * - * The single-segment directory patterns are leading-slash anchored - * (`/rt/`, not `rt/`): an unanchored `dir/` pattern matches that name at - * ANY depth in git, so a future tracked `user/rt/` would be silently - * ignored by a bare `rt/`. `*.sock` and `.DS_Store` are deliberately left - * unanchored — those are wanted at any depth. + * Post re-root, `user/` IS the personal repo — structure (nothing employer- + * adjacent or runtime lives inside it) is the security boundary, not this + * file. This list is ordinary hygiene: OS/transient noise that would + * otherwise get committed. All patterns are deliberately unanchored — they + * are wanted at any depth in the tree. */ -export const HOME_BOUNDARY: { tracked: string[]; ignored: string[] } = { - tracked: ["user/", "skills.jsonc", "snapshot-owners.jsonc", "user/secrets/"], - ignored: [ - "/rt/", - "/deck/", - "/shepherdr/", - "/repos/", - "/ci-attendants/", - "/work/", - "/teams/", - "user/local/", - "settings.local.jsonc", - "user/secrets/*.tmp", - "*.sock", - ".DS_Store", - ], +export const HOME_BOUNDARY: { ignored: string[] } = { + ignored: [".DS_Store", "*.sock", "*.tmp"], }; export function renderHomeGitignore(): string { diff --git a/lib/home/init-exec.ts b/lib/home/init-exec.ts index ed73a6f4..0ed30817 100644 --- a/lib/home/init-exec.ts +++ b/lib/home/init-exec.ts @@ -1,16 +1,14 @@ /** * `rt home init` execution — turns an InitStep[] (lib/home/init-plan.ts) into - * real git/gh/filter-repo calls, all routed through the injected ExecSeam so - * tests never touch a real subprocess or fs. + * real git/fs calls, all routed through the injected ExecSeam so tests never + * touch a real subprocess or fs. * * The seam is bound to the home directory: `run()` defaults its cwd to the - * home repo, and `writeFile`/`removeDir` take paths relative to it. Only the - * foldInPrefs temp clone overrides cwd explicitly. + * home root, and every other method takes paths relative to it. */ -import { mkdtempSync, rmSync, writeFileSync } from "fs"; -import { tmpdir } from "os"; -import { isAbsolute, join } from "path"; +import { mkdirSync, symlinkSync, unlinkSync, writeFileSync } from "fs"; +import { join } from "path"; import type { InitStep } from "./init-plan.ts"; export interface ExecResult { @@ -22,14 +20,13 @@ export interface ExecResult { export interface ExecSeam { run(cmd: string[], opts?: { cwd?: string }): Promise; writeFile(path: string, content: string): Promise; - removeDir(path: string): Promise; - mkTempDir(): Promise; + mkdirp(path: string): Promise; + /** Idempotent: replaces whatever (if anything) already sits at `path`. */ + writeSymlink(path: string, target: string): Promise; } export type InitResult = { ok: true } | { ok: false; failedStep: InitStep["kind"]; stderr: string }; -const FOLD_MERGE_MESSAGE = "home: fold in mattstack-prefs history under user/"; - type StepLog = (message: string) => void; class StepFailed extends Error { @@ -44,112 +41,52 @@ async function run(exec: ExecSeam, cmd: string[], opts?: { cwd?: string }): Prom return result.stdout; } -/** createRepo's stdout URL is carried forward for gitInit's `remote add`. */ -interface ExecContext { - createdRepoUrl?: string; -} - -async function runStep(step: InitStep, exec: ExecSeam, log: StepLog, ctx: ExecContext): Promise { +async function runStep(step: InitStep, exec: ExecSeam, log: StepLog): Promise { switch (step.kind) { - case "createRepo": { - // Resume-safe: a prior run may have already created the repo (e.g. it - // crashed on a later step). `gh repo view` tells us which of three - // states we're in before mutating anything. - log(`checking for an existing GitHub repo ${step.name}`); - const view = await exec.run(["gh", "repo", "view", step.name, "--json", "isEmpty,url"]); - if (view.code === 0) { - let parsed: { isEmpty?: boolean; url?: string }; - try { - parsed = JSON.parse(view.stdout); - } catch { - throw new StepFailed(`gh repo view printed unparseable JSON for ${step.name}`); - } - if (!parsed.isEmpty) { - throw new StepFailed(`GitHub repo "${step.name}" already exists and is not empty`); - } - if (!parsed.url) throw new StepFailed(`gh repo view printed no url for ${step.name}`); - ctx.createdRepoUrl = parsed.url; - return; + case "ensureStateDirs": { + for (const dir of step.dirs) { + log(`creating ${dir}/`); + await exec.mkdirp(dir); } - - log(`creating GitHub repo ${step.name}`); - const stdout = await run(exec, ["gh", "repo", "create", step.name, "--private"]); - const url = stdout.trim().split("\n")[0]; - // Silently skipping `remote add` here would surface as an unrelated - // failure six steps later, at push, with no way back to this step. - if (!url) throw new StepFailed("gh repo create printed no repo URL"); - ctx.createdRepoUrl = url; return; } - case "gitInit": { - log(`git init -b ${step.branch}`); - await run(exec, ["git", "init", "-b", step.branch]); - if (ctx.createdRepoUrl) await run(exec, ["git", "remote", "add", "origin", ctx.createdRepoUrl]); + case "cloneUserRepo": { + log(`cloning ${step.url} into user/`); + await run(exec, ["git", "clone", step.url, "user"]); return; } case "writeGitignore": { - log("writing the boundary .gitignore"); - await exec.writeFile(".gitignore", step.content); + log("writing the user repo's .gitignore"); + await exec.writeFile("user/.gitignore", step.content); return; } case "writeOwners": { - log("writing snapshot-owners.jsonc"); - await exec.writeFile("snapshot-owners.jsonc", step.content); + log("writing user/snapshot-owners.jsonc"); + await exec.writeFile("user/snapshot-owners.jsonc", step.content); return; } - case "deleteCruft": { - for (const path of step.paths) { - log(`removing stray cruft: ${path}`); - await exec.removeDir(path); - } + case "writeMachineKey": { + log(`writing machine-key (${step.key})`); + await exec.writeFile("machine-key", step.key); return; } - case "unlinkUserClone": { - // Must run before adoptCommit: see the ordering comment in - // init-plan.ts. foldInPrefs re-clones from step.sourceUrl instead, so - // this .git is never needed again. - log("unlinking user/.git (fold-in re-clones from the origin remote instead)"); - await exec.removeDir("user/.git"); + case "ensureProfileDir": { + log(`creating user/local/${step.key}/`); + await exec.mkdirp(join("user", "local", step.key)); return; } - case "foldInPrefs": { - log("folding mattstack-prefs history into user/"); - const tmp = await exec.mkTempDir(); - try { - // --no-hardlinks: if sourceUrl resolves to a local path, a plain - // clone would hardlink objects into the tmp clone; filter-repo - // rewrites history destructively, which would corrupt objects the - // source still shares. - await run(exec, ["git", "clone", "--no-hardlinks", step.sourceUrl, tmp]); - await run(exec, ["git", "filter-repo", "--to-subdirectory-filter", "user"], { cwd: tmp }); - // HEAD, not a hardcoded branch name: the tmp clone's default branch - // IS whatever the source remote's default branch is. - await run(exec, ["git", "fetch", tmp, "HEAD"]); - await run(exec, ["git", "merge", "FETCH_HEAD", "--allow-unrelated-histories", "-m", FOLD_MERGE_MESSAGE]); - } finally { - await exec.removeDir(tmp); - } - return; - } - case "adoptCommit": { - log(`committing: ${step.message}`); - await run(exec, ["git", "add", "-A"]); - await run(exec, ["git", "commit", "-m", step.message]); - return; - } - case "push": { - log(`pushing -u origin ${step.branch}`); - await run(exec, ["git", "push", "-u", "origin", step.branch]); + case "writeSkillsSymlink": { + log("linking skills.jsonc -> user/skills.jsonc"); + await exec.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); return; } } } export async function executeInitPlan(steps: InitStep[], exec: ExecSeam, log: StepLog): Promise { - const ctx: ExecContext = {}; for (const step of steps) { try { - await runStep(step, exec, log, ctx); + await runStep(step, exec, log); } catch (err) { const stderr = err instanceof StepFailed ? err.stderr : err instanceof Error ? err.message : String(err); return { ok: false, failedStep: step.kind, stderr }; @@ -158,7 +95,7 @@ export async function executeInitPlan(steps: InitStep[], exec: ExecSeam, log: St return { ok: true }; } -/** The real seam: Bun.spawn-based capture, real fs writes/removal under `home`. */ +/** The real seam: Bun.spawn-based capture, real fs writes under `home`. */ export function createRealExecSeam(home: string): ExecSeam { return { async run(cmd, opts) { @@ -181,13 +118,17 @@ export function createRealExecSeam(home: string): ExecSeam { async writeFile(path, content) { writeFileSync(join(home, path), content); }, - async removeDir(path) { - // The foldInPrefs temp clone is already absolute (outside `home`); - // every other caller passes a home-relative path. - rmSync(isAbsolute(path) ? path : join(home, path), { recursive: true, force: true }); + async mkdirp(path) { + mkdirSync(join(home, path), { recursive: true }); }, - async mkTempDir() { - return mkdtempSync(join(tmpdir(), "rt-home-fold-")); + async writeSymlink(path, target) { + const full = join(home, path); + try { + unlinkSync(full); + } catch { + // nothing there yet — the common case on a fresh machine + } + symlinkSync(target, full); }, }; } diff --git a/lib/home/init-plan.ts b/lib/home/init-plan.ts index dc8a52ab..a7a0e690 100644 --- a/lib/home/init-plan.ts +++ b/lib/home/init-plan.ts @@ -2,85 +2,85 @@ * `rt home init` plan-of-record. * * Pure logic: turns a probed HomeState into an ordered InitStep[]. No fs, no - * exec — a separate execution seam runs these against real git and gh. + * exec — a separate execution seam (lib/home/init-exec.ts) runs these + * against real git and the filesystem. */ import { renderHomeGitignore } from "./boundary.ts"; +/** ~/.mattstack state-zone directories: no repo, never travel. */ +export const STATE_DIR_NAMES = ["rt", "deck", "shepherdr", "repos", "work", "teams"]; + export interface HomeState { - isRepo: boolean; - hasUserClone: boolean; - hasTeamClones: string[]; - cruft: string[]; - /** origin URL of user/.git, parsed before it's ever unlinked. */ - prefsRemoteUrl?: string; + userRepoPresent: boolean; + machineKeyFilePresent: boolean; + profileDirPresent: boolean; + skillsSymlinkPresent: boolean; + /** A REAL file (not a symlink) already occupies the root skills.jsonc path. */ + skillsSymlinkBlocked: boolean; + stateDirsMissing: string[]; +} + +export interface InitPlanConfig { + url: string; + machineKey: string; } export type InitStep = - | { kind: "createRepo"; name: string } - | { kind: "gitInit"; branch: string } + | { kind: "ensureStateDirs"; dirs: string[] } + | { kind: "cloneUserRepo"; url: string } | { kind: "writeGitignore"; content: string } | { kind: "writeOwners"; content: string } - | { kind: "deleteCruft"; paths: string[] } - | { kind: "unlinkUserClone" } - | { kind: "foldInPrefs"; sourceUrl: string } - | { kind: "adoptCommit"; message: string } - | { kind: "push"; branch: string }; + | { kind: "writeMachineKey"; key: string } + | { kind: "ensureProfileDir"; key: string } + | { kind: "writeSkillsSymlink" }; export interface InitPlan { steps: InitStep[]; - /** - * Set only when the plan is empty: - * - "already-initialized": ~/.mattstack is already a repo. - * - "prefs-remote-unreadable": there's a user/ clone to fold in, but its - * origin URL couldn't be parsed — never emit a fold the executor can't - * actually run. - */ - reason?: "already-initialized" | "prefs-remote-unreadable"; + /** Set only when writeSkillsSymlink was omitted for skillsSymlinkBlocked — every other applicable step still runs. */ + blocked?: "skills-symlink-real-file"; } -export const DEFAULT_HOME_REPO_NAME = "mattstack-home"; -export const DEFAULT_HOME_BRANCH = "main"; -export const ADOPT_COMMIT_MESSAGE = "home: adopt the declarative layer"; - function renderOwnersFile(): string { return "{\n // snapshot-owners.jsonc — claimed zones the snapshot daemon must never\n // auto-commit. Empty until a zone is claimed.\n}\n"; } /** - * Idempotence lives here, not in the executor: a repo that already exists - * gets an empty plan so the executor never has to re-derive the check. + * Idempotence lives here, not in the executor: each step is gated on its own + * probe, so a fully-provisioned machine naturally converges to an empty + * plan without a special-cased short-circuit. */ -export function buildInitPlan(state: HomeState): InitPlan { - if (state.isRepo) return { steps: [], reason: "already-initialized" }; - if (state.hasUserClone && !state.prefsRemoteUrl) { - return { steps: [], reason: "prefs-remote-unreadable" }; +export function buildInitPlan(state: HomeState, config: InitPlanConfig): InitPlan { + const steps: InitStep[] = []; + + if (state.stateDirsMissing.length > 0) { + steps.push({ kind: "ensureStateDirs", dirs: state.stateDirsMissing }); } - const steps: InitStep[] = [ - { kind: "createRepo", name: DEFAULT_HOME_REPO_NAME }, - { kind: "gitInit", branch: DEFAULT_HOME_BRANCH }, - { kind: "writeGitignore", content: renderHomeGitignore() }, - { kind: "writeOwners", content: renderOwnersFile() }, - ]; + // writeGitignore/writeOwners ride along with the clone: an already-present + // user/ repo already carries these from its own history, so re-running + // init against it is provisioning-only and must not touch them. + if (!state.userRepoPresent) { + steps.push({ kind: "cloneUserRepo", url: config.url }); + steps.push({ kind: "writeGitignore", content: renderHomeGitignore() }); + steps.push({ kind: "writeOwners", content: renderOwnersFile() }); + } - if (state.cruft.length > 0) steps.push({ kind: "deleteCruft", paths: state.cruft }); + if (!state.machineKeyFilePresent) { + steps.push({ kind: "writeMachineKey", key: config.machineKey }); + } - // unlinkUserClone runs BEFORE adoptCommit: a live user/.git left in place - // makes `git add -A` stage `user` as a GITLINK (mode 160000), not the real - // files under it — the fold-in merge below then sees those files as - // untracked and refuses. Removing .git first makes `user/**` ordinary - // tracked files, so the later merge is a clean add/add of identical blobs. - if (state.hasUserClone) steps.push({ kind: "unlinkUserClone" }); + if (!state.profileDirPresent) { + steps.push({ kind: "ensureProfileDir", key: config.machineKey }); + } - // adoptCommit runs BEFORE foldInPrefs: folding merges FETCH_HEAD with - // --allow-unrelated-histories, and a merge into a still-unborn HEAD (no - // commits yet) refuses to clobber the untracked user/ files already on - // disk. Committing first turns that merge into a clean 3-way add/add. - steps.push({ kind: "adoptCommit", message: ADOPT_COMMIT_MESSAGE }); - if (state.hasUserClone) steps.push({ kind: "foldInPrefs", sourceUrl: state.prefsRemoteUrl! }); + if (state.skillsSymlinkBlocked) { + return { steps, blocked: "skills-symlink-real-file" }; + } - steps.push({ kind: "push", branch: DEFAULT_HOME_BRANCH }); + if (!state.skillsSymlinkPresent) { + steps.push({ kind: "writeSkillsSymlink" }); + } return { steps }; } diff --git a/lib/secrets/__tests__/store.test.ts b/lib/secrets/__tests__/store.test.ts index b93746c2..255972df 100644 --- a/lib/secrets/__tests__/store.test.ts +++ b/lib/secrets/__tests__/store.test.ts @@ -549,7 +549,7 @@ describe("rotateSecret", () => { }); }); -describe("real seam spawn options — cwd pin (Task 5 carried review item)", () => { +describe("real seam spawn options — cwd pin", () => { test("pins cwd to /user so sops resolves THIS home's .sops.yaml (and secrets/.* regex), never a foreign cwd's", () => { const opts = buildSecretsSpawnOptions(); expect(opts.cwd).toBe(join(mattstackHome(), "user")); From e261adc6d686f3ea9568e06aa91d899831effed0 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 13:26:24 -0500 Subject: [PATCH 5/9] =?UTF-8?q?RT:=20review=20fixes=20=E2=80=94=20exec-tim?= =?UTF-8?q?e=20re-checks,=20machine-key=20guard,=20url=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings 1-2 (data-clobber): writeSkillsSymlink now re-checks isRealFile at exec time instead of trusting the plan-build-time probe (lstat, not trust) — a real file at the root path fails the step instead of being unlinked. writeGitignore/writeOwners move to write-if-absent, decided at exec time via a new ExecSeam.exists(): the plan can't know whether a fresh clone will land populated or empty, so only the executor can decide; a populated clone's tracked files are left untouched. Finding 3: extracts isSafeMachineKeySegment from rt-paths.ts (mirrored in rt-client's paths.ts, dist rebuilt) as the one guard shared by machineKey()'s override check and buildInitPlan, which now throws InvalidMachineKeyError for a key that would fail it — refused before it can ever be written and silently rejected on the next read. Finding 4: "already fully provisioned" now gates on !plan.blocked, so it never prints alongside the skills-symlink-blocked error. Finding 5: parseUrlArg rejects a missing --url value or one that looks like another flag (--url --dry-run) instead of silently defaulting or swallowing the next flag. Finding 6: machineSettingsPath's doc comment (both rt-paths.ts and the rt-client mirror) no longer claims machine scope is untracked/gitignored — ruling 2 made it tracked and keyed per machine. Ruling 8a: deletes lib/home/git-config.ts (parseOriginUrl) and its test — no production caller since the prefs-fold flow retired. Ruling 8b: adds ci-attendants to STATE_DIR_NAMES per the spec's state-zone tree. --- commands/__tests__/home.test.ts | 103 ++++++++++- commands/home.ts | 45 ++++- lib/__tests__/rt-paths.test.ts | 15 +- lib/home/__tests__/git-config.test.ts | 56 ------ lib/home/__tests__/init-exec.test.ts | 207 ++++++++++++++++++++--- lib/home/__tests__/init-plan.test.ts | 28 ++- lib/home/git-config.ts | 30 ---- lib/home/init-exec.ts | 41 ++++- lib/home/init-plan.ts | 19 ++- lib/rt-paths.ts | 22 ++- packages/rt-client/src/settings/paths.ts | 9 +- 11 files changed, 440 insertions(+), 135 deletions(-) delete mode 100644 lib/home/__tests__/git-config.test.ts delete mode 100644 lib/home/git-config.ts diff --git a/commands/__tests__/home.test.ts b/commands/__tests__/home.test.ts index 6daa994e..f4994407 100644 --- a/commands/__tests__/home.test.ts +++ b/commands/__tests__/home.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect, spyOn } from "bun:test"; -import { DEFAULT_USER_REPO_URL, gatherHomeState, homeInit, type HomeProbes, type SopsYamlSeam } from "../home.ts"; +import { DEFAULT_USER_REPO_URL, gatherHomeState, homeInit, InvalidUrlArgError, type HomeProbes, type SopsYamlSeam } from "../home.ts"; import { STATE_DIR_NAMES } from "../../lib/home/init-plan.ts"; import type { ExecResult, ExecSeam } from "../../lib/home/init-exec.ts"; import { renderSopsYaml, type AgeExecResult, type AgeKeySeam } from "../../lib/home/age-key.ts"; @@ -130,6 +130,14 @@ class FakeSeam implements ExecSeam { async mkdirp(path: string): Promise { this.calls.push({ kind: "mkdirp", arg: path }); } + async exists(path: string): Promise { + this.calls.push({ kind: "exists", arg: path }); + return false; + } + async isRealFile(path: string): Promise { + this.calls.push({ kind: "isRealFile", arg: path }); + return false; + } async writeSymlink(path: string, target: string): Promise { this.calls.push({ kind: "writeSymlink", arg: { path, target } }); } @@ -300,4 +308,97 @@ describe("homeInit", () => { expect(seam.calls).toEqual([]); expect(ageKeySeam.calls).toEqual([]); }); + + test("fully provisioned but a real file blocks the symlink: never prints 'fully provisioned', still reports the block", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + // Every state-dir/machine-key/profile-dir/user-repo probe reports + // present, so the plan is empty except for the block — exactly the + // "steps.length === 0 AND blocked" case the fully-provisioned message + // must not fire on. + const probes = fakeProbes({ + isGitRepo: (dir) => dir.endsWith("/user"), + exists: () => true, + readSymlinkTarget: () => null, + }); + + const { exitCode, logs, errors } = await runHomeInit(probes, seam, ageKeySeam); + + expect(exitCode).toBe(1); + expect(logs.some((l) => l.includes("already fully provisioned"))).toBe(false); + expect(errors.some((e) => e.includes("refusing to overwrite"))).toBe(true); + }); + + test("fully provisioned but blocked, --dry-run: still never prints 'fully provisioned'", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + const probes = fakeProbes({ + isGitRepo: (dir) => dir.endsWith("/user"), + exists: () => true, + readSymlinkTarget: () => null, + }); + + const { exitCode, logs, errors } = await runHomeInit(probes, seam, ageKeySeam, ["--dry-run"]); + + expect(exitCode).toBeUndefined(); + expect(logs.some((l) => l.includes("already fully provisioned"))).toBe(false); + expect(errors.some((e) => e.includes("refusing to overwrite"))).toBe(true); + }); + + describe("--url validation", () => { + test("--url as the last arg (no value): exits 1 with a clear error, runs nothing", async () => { + const seam = new FakeSeam(); + const { exitCode, errors } = await runHomeInit(fakeProbes({}), seam, new FakeAgeKeySeam(), ["--url"]); + + expect(exitCode).toBe(1); + expect(errors.some((e) => e.includes("--url requires a value"))).toBe(true); + expect(seam.calls).toEqual([]); + }); + + test("--url followed by another flag: refuses to take the flag as the URL", async () => { + const seam = new FakeSeam(); + const { exitCode, errors } = await runHomeInit(fakeProbes({}), seam, new FakeAgeKeySeam(), ["--url", "--dry-run"]); + + expect(exitCode).toBe(1); + expect(errors.some((e) => e.includes("--url requires a value"))).toBe(true); + expect(seam.calls).toEqual([]); + }); + + test("parseUrlArg's error type is exported and matches what homeInit catches", () => { + expect(new InvalidUrlArgError("x")).toBeInstanceOf(Error); + }); + }); + + describe("machine-key safety guard", () => { + test("an injected key that fails the safety guard: exits 1 with a clear error, runs no step", async () => { + const seam = new FakeSeam(); + const { exitCode, errors } = await runHomeInit( + fakeProbes({}), + seam, + new FakeAgeKeySeam(), + [], + new FakeSopsYamlSeam(), + "../escape", + ); + + expect(exitCode).toBe(1); + expect(errors.some((e) => e.includes("not a safe machine-key segment"))).toBe(true); + expect(seam.calls).toEqual([]); + }); + + test("an injected key with a path separator: exits 1, never reaches ensureProfileDir/writeMachineKey", async () => { + const seam = new FakeSeam(); + const { exitCode } = await runHomeInit( + fakeProbes({}), + seam, + new FakeAgeKeySeam(), + [], + new FakeSopsYamlSeam(), + "evil/key", + ); + + expect(exitCode).toBe(1); + expect(seam.calls.some((c) => c.kind === "mkdirp" || c.kind === "writeFile")).toBe(false); + }); + }); }); diff --git a/commands/home.ts b/commands/home.ts index e90f3c4e..fe1d38b1 100644 --- a/commands/home.ts +++ b/commands/home.ts @@ -14,7 +14,7 @@ import { existsSync, readFileSync, readlinkSync, writeFileSync } from "fs"; import { join } from "path"; import type { CommandContext } from "../lib/command-tree.ts"; import { machineKey, mattstackHome } from "../lib/rt-paths.ts"; -import { buildInitPlan, STATE_DIR_NAMES, type HomeState, type InitStep } from "../lib/home/init-plan.ts"; +import { buildInitPlan, InvalidMachineKeyError, STATE_DIR_NAMES, type HomeState, type InitStep } from "../lib/home/init-plan.ts"; import { createRealExecSeam, executeInitPlan, type ExecSeam } from "../lib/home/init-exec.ts"; import { AgeKeyAbsentError, @@ -110,10 +110,18 @@ function describeStep(step: InitStep): string { } } +/** Thrown by parseUrlArg for a `--url` with no usable value — never silently absorbed into the default or into the next flag. */ +export class InvalidUrlArgError extends Error {} + function parseUrlArg(args: string[]): string { const idx = args.indexOf("--url"); - const value = idx !== -1 ? args[idx + 1] : undefined; - return value && value.length > 0 ? value : DEFAULT_USER_REPO_URL; + if (idx === -1) return DEFAULT_USER_REPO_URL; + + const value = args[idx + 1]; + if (value === undefined || value.startsWith("--")) { + throw new InvalidUrlArgError("--url requires a value, e.g. --url https://github.com/org/mattstack-home"); + } + return value; } /** @@ -169,15 +177,36 @@ export async function homeInit( ): Promise { const dryRun = args.includes("--dry-run"); const home = mattstackHome(); - const url = parseUrlArg(args); + + let url: string; + try { + url = parseUrlArg(args); + } catch (err) { + if (err instanceof InvalidUrlArgError) { + console.error(`rt home init: ${err.message}`); + process.exit(1); + } + throw err; + } + const state = gatherHomeState(home, probes, key); - const plan = buildInitPlan(state, { url, machineKey: key }); - if (plan.steps.length === 0) { - console.log(`rt home init: ${home} is already fully provisioned — nothing to do.`); - } else { + let plan: ReturnType; + try { + plan = buildInitPlan(state, { url, machineKey: key }); + } catch (err) { + if (err instanceof InvalidMachineKeyError) { + console.error(`rt home init: ${err.message}`); + process.exit(1); + } + throw err; + } + + if (plan.steps.length > 0) { console.log(`rt home init plan for ${home}:`); plan.steps.forEach((step, i) => console.log(` ${i + 1}. ${describeStep(step)}`)); + } else if (!plan.blocked) { + console.log(`rt home init: ${home} is already fully provisioned — nothing to do.`); } if (plan.blocked === "skills-symlink-real-file") { diff --git a/lib/__tests__/rt-paths.test.ts b/lib/__tests__/rt-paths.test.ts index a8ef5f2f..bea49dcb 100644 --- a/lib/__tests__/rt-paths.test.ts +++ b/lib/__tests__/rt-paths.test.ts @@ -30,7 +30,7 @@ import { migrateLegacyRtDir, legacyDirsPresent, TRAY_APP_NAME, DEV_TRAY_APP_NAME, TRAY_APP_BUNDLE, DEV_TRAY_APP_BUNDLE, trayAppPath, devTrayAppPath, legacyTrayAppPaths, installedTrayAppPath, machineSettingsPath, - machineKey, userSettingsPath, teamSettingsPath, + machineKey, userSettingsPath, teamSettingsPath, isSafeMachineKeySegment, } from "../rt-paths.ts"; describe("rt-paths", () => { @@ -176,6 +176,19 @@ describe("rt-paths", () => { }); }); + describe("isSafeMachineKeySegment", () => { + test.each([ + ["a plain slug", "mbp-14", true], + ["empty", "", false], + ["exactly \".\"", ".", false], + ["exactly \"..\"", "..", false], + ["a forward slash", "evil/key", false], + ["a backslash", "evil\\key", false], + ])("%s -> %s", (_label, value, expected) => { + expect(isSafeMachineKeySegment(value)).toBe(expected); + }); + }); + // ── migrateLegacyRtDir ─────────────────────────────────────────────────────── const makeHome = () => mkdtempSync(join(tmpdir(), "rt-paths-migrate-")); diff --git a/lib/home/__tests__/git-config.test.ts b/lib/home/__tests__/git-config.test.ts deleted file mode 100644 index 78aa7a8e..00000000 --- a/lib/home/__tests__/git-config.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { describe, test, expect } from "bun:test"; -import { parseOriginUrl } from "../git-config.ts"; - -describe("parseOriginUrl", () => { - test("extracts the origin remote url from a realistic clone config", () => { - const config = `[core] -\trepositoryformatversion = 0 -\tfilemode = true -\tbare = false -\tlogallrefupdates = true -[remote "origin"] -\turl = https://github.com/mattgoodwin/mattstack-prefs.git -\tfetch = +refs/heads/*:refs/remotes/origin/* -[branch "main"] -\tremote = origin -\tmerge = refs/heads/main -`; - - expect(parseOriginUrl(config)).toBe("https://github.com/mattgoodwin/mattstack-prefs.git"); - }); - - test("extracts an ssh-form url", () => { - const config = `[remote "origin"] -\turl = git@github.com:mattgoodwin/mattstack-prefs.git -\tfetch = +refs/heads/*:refs/remotes/origin/* -`; - expect(parseOriginUrl(config)).toBe("git@github.com:mattgoodwin/mattstack-prefs.git"); - }); - - test("ignores a url line belonging to a different remote", () => { - const config = `[remote "upstream"] -\turl = https://github.com/someone-else/mattstack-prefs.git -[remote "origin"] -\turl = https://github.com/mattgoodwin/mattstack-prefs.git -`; - expect(parseOriginUrl(config)).toBe("https://github.com/mattgoodwin/mattstack-prefs.git"); - }); - - test("returns null when there is no [remote \"origin\"] section", () => { - const config = `[core] -\trepositoryformatversion = 0 -`; - expect(parseOriginUrl(config)).toBeNull(); - }); - - test("returns null for an empty file", () => { - expect(parseOriginUrl("")).toBeNull(); - }); - - test("returns null when the origin section has no url line", () => { - const config = `[remote "origin"] -\tfetch = +refs/heads/*:refs/remotes/origin/* -`; - expect(parseOriginUrl(config)).toBeNull(); - }); -}); diff --git a/lib/home/__tests__/init-exec.test.ts b/lib/home/__tests__/init-exec.test.ts index c2790500..42d66bb1 100644 --- a/lib/home/__tests__/init-exec.test.ts +++ b/lib/home/__tests__/init-exec.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { existsSync, lstatSync, mkdtempSync, readFileSync, readlinkSync, realpathSync, rmSync, writeFileSync } from "fs"; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, realpathSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { createRealExecSeam, executeInitPlan, type ExecResult, type ExecSeam } from "../init-exec.ts"; @@ -11,6 +11,8 @@ type RecordedCall = | { kind: "run"; cmd: string[]; cwd?: string } | { kind: "writeFile"; path: string; content: string } | { kind: "mkdirp"; path: string } + | { kind: "exists"; path: string } + | { kind: "isRealFile"; path: string } | { kind: "writeSymlink"; path: string; target: string }; const noopLog = () => {}; @@ -19,7 +21,13 @@ const noopLog = () => {}; class FakeExecSeam implements ExecSeam { calls: RecordedCall[] = []; - constructor(private opts: { failRun?: (cmd: string[]) => string | undefined } = {}) {} + constructor( + private opts: { + failRun?: (cmd: string[]) => string | undefined; + exists?: (path: string) => boolean; + isRealFile?: (path: string) => boolean; + } = {}, + ) {} async run(cmd: string[], runOpts?: { cwd?: string }): Promise { this.calls.push({ kind: "run", cmd, cwd: runOpts?.cwd }); @@ -36,6 +44,16 @@ class FakeExecSeam implements ExecSeam { this.calls.push({ kind: "mkdirp", path }); } + async exists(path: string): Promise { + this.calls.push({ kind: "exists", path }); + return this.opts.exists?.(path) ?? false; + } + + async isRealFile(path: string): Promise { + this.calls.push({ kind: "isRealFile", path }); + return this.opts.isRealFile?.(path) ?? false; + } + async writeSymlink(path: string, target: string): Promise { this.calls.push({ kind: "writeSymlink", path, target }); } @@ -78,20 +96,41 @@ describe("executeInitPlan", () => { } }); - test("writeGitignore and writeOwners write into the user repo, not the root", async () => { - const seam = new FakeExecSeam(); - const steps: InitStep[] = [ - { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, - { kind: "writeOwners", content: "{}\n" }, - ]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([ - { kind: "writeFile", path: "user/.gitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, - { kind: "writeFile", path: "user/snapshot-owners.jsonc", content: "{}\n" }, - ]); + describe("writeGitignore / writeOwners — write-if-absent, decided at exec time", () => { + test("an empty (freshly created) clone: neither file exists yet, so the ruled content is written", async () => { + const seam = new FakeExecSeam({ exists: () => false }); + const steps: InitStep[] = [ + { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeOwners", content: "{}\n" }, + ]; + + const result = await executeInitPlan(steps, seam, noopLog); + + expect(result).toEqual({ ok: true }); + expect(seam.calls).toEqual([ + { kind: "exists", path: "user/.gitignore" }, + { kind: "writeFile", path: "user/.gitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "exists", path: "user/snapshot-owners.jsonc" }, + { kind: "writeFile", path: "user/snapshot-owners.jsonc", content: "{}\n" }, + ]); + }); + + test("a populated clone: both files already exist (brought by the clone's own history) — left untouched", async () => { + const seam = new FakeExecSeam({ exists: () => true }); + const steps: InitStep[] = [ + { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeOwners", content: "{}\n" }, + ]; + + const result = await executeInitPlan(steps, seam, noopLog); + + expect(result).toEqual({ ok: true }); + expect(seam.calls).toEqual([ + { kind: "exists", path: "user/.gitignore" }, + { kind: "exists", path: "user/snapshot-owners.jsonc" }, + ]); + expect(seam.calls.some((c) => c.kind === "writeFile")).toBe(false); + }); }); test("writeMachineKey: writes the key to the root machine-key file", async () => { @@ -114,18 +153,38 @@ describe("executeInitPlan", () => { expect(seam.calls).toEqual([{ kind: "mkdirp", path: join("user", "local", "mbp-14") }]); }); - test("writeSkillsSymlink: links the root path to user/skills.jsonc", async () => { - const seam = new FakeExecSeam(); - const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; - - const result = await executeInitPlan(steps, seam, noopLog); - - expect(result).toEqual({ ok: true }); - expect(seam.calls).toEqual([{ kind: "writeSymlink", path: "skills.jsonc", target: join("user", "skills.jsonc") }]); + describe("writeSkillsSymlink — re-checked at exec time, never trusts the plan-build-time probe", () => { + test("nothing (or a symlink) at the root path: links skills.jsonc -> user/skills.jsonc", async () => { + const seam = new FakeExecSeam({ isRealFile: () => false }); + const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; + + const result = await executeInitPlan(steps, seam, noopLog); + + expect(result).toEqual({ ok: true }); + expect(seam.calls).toEqual([ + { kind: "isRealFile", path: "skills.jsonc" }, + { kind: "writeSymlink", path: "skills.jsonc", target: join("user", "skills.jsonc") }, + ]); + }); + + test("a REAL file at the root path: the step fails, and writeSymlink (so unlink) is never called", async () => { + const seam = new FakeExecSeam({ isRealFile: () => true }); + const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; + + const result = await executeInitPlan(steps, seam, noopLog); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.failedStep).toBe("writeSkillsSymlink"); + expect(result.stderr).toContain("refusing to overwrite"); + } + expect(seam.calls).toEqual([{ kind: "isRealFile", path: "skills.jsonc" }]); + expect(seam.calls.some((c) => c.kind === "writeSymlink")).toBe(false); + }); }); test("runs a full fresh-machine plan's steps in order", async () => { - const seam = new FakeExecSeam(); + const seam = new FakeExecSeam({ exists: () => false, isRealFile: () => false }); const steps = buildInitPlan( { userRepoPresent: false, @@ -146,13 +205,17 @@ describe("executeInitPlan", () => { "mkdirp", // deck "mkdirp", // shepherdr "mkdirp", // repos + "mkdirp", // ci-attendants "mkdirp", // work "mkdirp", // teams "run", // git clone + "exists", // user/.gitignore "writeFile", // user/.gitignore + "exists", // user/snapshot-owners.jsonc "writeFile", // user/snapshot-owners.jsonc "writeFile", // machine-key "mkdirp", // user/local/mbp-14 + "isRealFile", // skills.jsonc "writeSymlink", // skills.jsonc ]); }); @@ -178,7 +241,7 @@ describe("executeInitPlan", () => { }); describe("createRealExecSeam", () => { - test("run() defaults cwd to home; mkdirp/writeFile/writeSymlink resolve relative to home", async () => { + test("run() defaults cwd to home; mkdirp/writeFile/exists/writeSymlink resolve relative to home", async () => { const home = mkdtempSync(join(tmpdir(), "rt-home-exec-test-")); try { const seam = createRealExecSeam(home); @@ -190,13 +253,17 @@ describe("createRealExecSeam", () => { await seam.mkdirp(join("user", "local", "mbp-14")); expect(existsSync(join(home, "user", "local", "mbp-14"))).toBe(true); + expect(await seam.exists("user/skills.jsonc")).toBe(false); await seam.writeFile("user/skills.jsonc", "{}\n"); expect(readFileSync(join(home, "user", "skills.jsonc"), "utf8")).toBe("{}\n"); + expect(await seam.exists("user/skills.jsonc")).toBe(true); + expect(await seam.isRealFile("skills.jsonc")).toBe(false); // absent await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); const st = lstatSync(join(home, "skills.jsonc")); expect(st.isSymbolicLink()).toBe(true); expect(readlinkSync(join(home, "skills.jsonc"))).toBe(join("user", "skills.jsonc")); + expect(await seam.isRealFile("skills.jsonc")).toBe(false); // a symlink, not a real file // writeSymlink replaces whatever was already there. await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); @@ -205,4 +272,92 @@ describe("createRealExecSeam", () => { rmSync(home, { recursive: true, force: true }); } }); + + // The reviewer proved the clobber live: writeSymlink alone unconditionally + // unlinked a real skills.jsonc. isRealFile is the guard runStep checks + // FIRST — this proves it against a genuine real file, not a fake. + test("isRealFile is true for a genuine file, distinguishing it from a symlink", async () => { + const home = mkdtempSync(join(tmpdir(), "rt-home-exec-realfile-")); + try { + const seam = createRealExecSeam(home); + writeFileSync(join(home, "skills.jsonc"), '{"real": "content"}\n'); + + expect(await seam.isRealFile("skills.jsonc")).toBe(true); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + test("executeInitPlan(writeSkillsSymlink) against a real skills.jsonc file: fails, leaves the file byte-identical, creates no symlink", async () => { + const home = mkdtempSync(join(tmpdir(), "rt-home-exec-clobber-guard-")); + try { + const original = '{"real": "content", "do-not-touch": true}\n'; + writeFileSync(join(home, "skills.jsonc"), original); + const seam = createRealExecSeam(home); + + const result = await executeInitPlan([{ kind: "writeSkillsSymlink" }], seam, noopLog); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.failedStep).toBe("writeSkillsSymlink"); + expect(result.stderr).toContain("refusing to overwrite"); + } + const st = lstatSync(join(home, "skills.jsonc")); + expect(st.isSymbolicLink()).toBe(false); + expect(readFileSync(join(home, "skills.jsonc"), "utf8")).toBe(original); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + test("writeGitignore/writeOwners against a real, already-populated clone: left byte-identical", async () => { + const home = mkdtempSync(join(tmpdir(), "rt-home-exec-populated-clone-")); + try { + const seam = createRealExecSeam(home); + const userDir = join(home, "user"); + mkdirSync(userDir, { recursive: true }); + const existingGitignore = "# hand-curated, from the clone's own history\n"; + const existingOwners = '{ "claimview": "matt" }\n'; + writeFileSync(join(userDir, ".gitignore"), existingGitignore); + writeFileSync(join(userDir, "snapshot-owners.jsonc"), existingOwners); + + const result = await executeInitPlan( + [ + { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeOwners", content: "{}\n" }, + ], + seam, + noopLog, + ); + + expect(result).toEqual({ ok: true }); + expect(readFileSync(join(userDir, ".gitignore"), "utf8")).toBe(existingGitignore); + expect(readFileSync(join(userDir, "snapshot-owners.jsonc"), "utf8")).toBe(existingOwners); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + test("writeGitignore/writeOwners against a genuinely empty clone: seeds the ruled content", async () => { + const home = mkdtempSync(join(tmpdir(), "rt-home-exec-empty-clone-")); + try { + const seam = createRealExecSeam(home); + mkdirSync(join(home, "user"), { recursive: true }); + + const result = await executeInitPlan( + [ + { kind: "writeGitignore", content: ".DS_Store\n*.sock\n*.tmp\n" }, + { kind: "writeOwners", content: "{}\n" }, + ], + seam, + noopLog, + ); + + expect(result).toEqual({ ok: true }); + expect(readFileSync(join(home, "user", ".gitignore"), "utf8")).toBe(".DS_Store\n*.sock\n*.tmp\n"); + expect(readFileSync(join(home, "user", "snapshot-owners.jsonc"), "utf8")).toBe("{}\n"); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); }); diff --git a/lib/home/__tests__/init-plan.test.ts b/lib/home/__tests__/init-plan.test.ts index 32e29b44..e290e7bb 100644 --- a/lib/home/__tests__/init-plan.test.ts +++ b/lib/home/__tests__/init-plan.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { buildInitPlan, STATE_DIR_NAMES, type HomeState, type InitPlanConfig } from "../init-plan.ts"; +import { buildInitPlan, InvalidMachineKeyError, STATE_DIR_NAMES, type HomeState, type InitPlanConfig } from "../init-plan.ts"; const CONFIG: InitPlanConfig = { url: "https://github.com/m4ttheweric/mattstack-home", machineKey: "mbp-14" }; @@ -134,4 +134,30 @@ describe("buildInitPlan", () => { expect(plan.steps).toEqual([]); expect(plan.blocked).toBe("skills-symlink-real-file"); }); + + test("STATE_DIR_NAMES includes ci-attendants (per the spec's state-zone tree)", () => { + expect(STATE_DIR_NAMES).toContain("ci-attendants"); + }); + + describe("machine-key guard — refuses before ever emitting writeMachineKey/ensureProfileDir", () => { + test.each([ + ["empty", ""], + ["exactly \".\"", "."], + ["exactly \"..\"", ".."], + ["a forward slash", "evil/key"], + ["a backslash", "evil\\key"], + ])("%s: throws InvalidMachineKeyError, never returns a plan", (_label, badKey) => { + expect(() => buildInitPlan(FRESH_STATE, { ...CONFIG, machineKey: badKey })).toThrow(InvalidMachineKeyError); + }); + + test("a safe key still builds the plan normally", () => { + expect(() => buildInitPlan(FRESH_STATE, { ...CONFIG, machineKey: "mbp-14" })).not.toThrow(); + }); + + test("the guard applies even when nothing else in the plan needs the key (fully provisioned)", () => { + expect(() => buildInitPlan(FULLY_PROVISIONED_STATE, { ...CONFIG, machineKey: "../escape" })).toThrow( + InvalidMachineKeyError, + ); + }); + }); }); diff --git a/lib/home/git-config.ts b/lib/home/git-config.ts deleted file mode 100644 index d2717ceb..00000000 --- a/lib/home/git-config.ts +++ /dev/null @@ -1,30 +0,0 @@ -/** - * Pure `.git/config` parsing — no fs, no exec. Used to recover the origin - * remote URL of a clone whose `.git` is about to be (or already was) - * unlinked, so a caller doesn't need `git remote get-url` (a subprocess) to - * answer a question the config file already holds as text. - */ - -/** - * Extracts the `url` under `[remote "origin"]`. Only that one section is - * read — `git config`'s full include/multi-value semantics don't apply here, - * since this reads a single known-shape file rather than resolving a real - * git config graph. - */ -export function parseOriginUrl(gitConfigText: string): string | null { - const lines = gitConfigText.split("\n"); - let inOriginSection = false; - - for (const rawLine of lines) { - const line = rawLine.trim(); - if (line.startsWith("[")) { - inOriginSection = /^\[remote\s+"origin"\]$/.test(line); - continue; - } - if (!inOriginSection) continue; - const match = line.match(/^url\s*=\s*(.+)$/); - if (match) return match[1]!.trim(); - } - - return null; -} diff --git a/lib/home/init-exec.ts b/lib/home/init-exec.ts index 0ed30817..6d1db061 100644 --- a/lib/home/init-exec.ts +++ b/lib/home/init-exec.ts @@ -7,7 +7,7 @@ * home root, and every other method takes paths relative to it. */ -import { mkdirSync, symlinkSync, unlinkSync, writeFileSync } from "fs"; +import { existsSync, lstatSync, mkdirSync, symlinkSync, unlinkSync, writeFileSync } from "fs"; import { join } from "path"; import type { InitStep } from "./init-plan.ts"; @@ -21,7 +21,11 @@ export interface ExecSeam { run(cmd: string[], opts?: { cwd?: string }): Promise; writeFile(path: string, content: string): Promise; mkdirp(path: string): Promise; - /** Idempotent: replaces whatever (if anything) already sits at `path`. */ + /** Any entry at all (file, dir, or symlink) — used to seed tracked files only into a genuinely empty clone. */ + exists(path: string): Promise; + /** True only for a REAL (non-symlink) entry — absent or a symlink both read false. */ + isRealFile(path: string): Promise; + /** Idempotent: replaces whatever (if anything) already sits at `path`. Callers must confirm via isRealFile first — this never itself refuses a real file. */ writeSymlink(path: string, target: string): Promise; } @@ -41,6 +45,16 @@ async function run(exec: ExecSeam, cmd: string[], opts?: { cwd?: string }): Prom return result.stdout; } +/** Seeds a tracked file only into a genuinely empty clone — a populated clone already carries it from its own history. */ +async function writeIfAbsent(exec: ExecSeam, path: string, content: string, log: StepLog, label: string): Promise { + if (await exec.exists(path)) { + log(`${label} already present — leaving it`); + return; + } + log(`seeding ${label}`); + await exec.writeFile(path, content); +} + async function runStep(step: InitStep, exec: ExecSeam, log: StepLog): Promise { switch (step.kind) { case "ensureStateDirs": { @@ -56,13 +70,11 @@ async function runStep(step: InitStep, exec: ExecSeam, log: StepLog): Promise user/skills.jsonc"); await exec.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); return; @@ -121,6 +140,16 @@ export function createRealExecSeam(home: string): ExecSeam { async mkdirp(path) { mkdirSync(join(home, path), { recursive: true }); }, + async exists(path) { + return existsSync(join(home, path)); + }, + async isRealFile(path) { + try { + return !lstatSync(join(home, path)).isSymbolicLink(); + } catch { + return false; // absent + } + }, async writeSymlink(path, target) { const full = join(home, path); try { diff --git a/lib/home/init-plan.ts b/lib/home/init-plan.ts index a7a0e690..cc9a49ec 100644 --- a/lib/home/init-plan.ts +++ b/lib/home/init-plan.ts @@ -6,10 +6,11 @@ * against real git and the filesystem. */ +import { isSafeMachineKeySegment } from "../rt-paths.ts"; import { renderHomeGitignore } from "./boundary.ts"; /** ~/.mattstack state-zone directories: no repo, never travel. */ -export const STATE_DIR_NAMES = ["rt", "deck", "shepherdr", "repos", "work", "teams"]; +export const STATE_DIR_NAMES = ["rt", "deck", "shepherdr", "repos", "ci-attendants", "work", "teams"]; export interface HomeState { userRepoPresent: boolean; @@ -45,12 +46,28 @@ function renderOwnersFile(): string { return "{\n // snapshot-owners.jsonc — claimed zones the snapshot daemon must never\n // auto-commit. Empty until a zone is claimed.\n}\n"; } +/** A machine-key value that would fail machineKey()'s own override guard — refused before it can ever be written and then silently ignored. */ +export class InvalidMachineKeyError extends Error { + constructor(key: string) { + super(`"${key}" is not a safe machine-key segment (empty, ".", "..", or containing "/" or "\\")`); + } +} + /** * Idempotence lives here, not in the executor: each step is gated on its own * probe, so a fully-provisioned machine naturally converges to an empty * plan without a special-cased short-circuit. */ export function buildInitPlan(state: HomeState, config: InitPlanConfig): InitPlan { + // Refused here, not left to machineKey()'s own guard: a bad key that + // slipped through would get written to disk by writeMachineKey and then + // silently rejected on the next read, stranding ensureProfileDir's + // user/local// as a dead directory while the resolver quietly falls + // back to the hostname slug. + if (!isSafeMachineKeySegment(config.machineKey)) { + throw new InvalidMachineKeyError(config.machineKey); + } + const steps: InitStep[] = []; if (state.stateDirsMissing.length > 0) { diff --git a/lib/rt-paths.ts b/lib/rt-paths.ts index b24fcf90..b6f946a5 100644 --- a/lib/rt-paths.ts +++ b/lib/rt-paths.ts @@ -95,8 +95,9 @@ export function teamSettingsPath(team: string): string { /** * ~/.mattstack/user/local//settings.local.jsonc — the machine - * store: local overrides, never committed or synced (`user/local/` is - * gitignored in the home repo). Nested per machine so multiple machines + * store: local overrides, TRACKED and keyed per machine (ruling 2 — machine + * scope travels, it just never collides, since each machine only ever + * writes its own `local//`). Nested per machine so multiple machines * sharing the synced `user/` tree don't collide on one local-overrides file. */ export function machineSettingsPath(): string { @@ -130,7 +131,7 @@ export function machineKey(): string { const override = join(home(), ".mattstack", "machine-key"); try { const v = readFileSync(override, "utf8").trim(); - if (v && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\")) return v; + if (isSafeMachineKeySegment(v)) return v; } catch { // no override file — fall through to the hostname slug } @@ -142,6 +143,21 @@ export function machineKey(): string { return slug || "default"; } +/** + * The one guard for "is this string safe to use as a `user/local//` + * directory name" — shared by machineKey()'s override check and + * lib/home/init-plan.ts's buildInitPlan (which refuses to emit + * writeMachineKey/ensureProfileDir for a key that would fail this, rather + * than write a file the resolver's own override check would then reject, + * silently falling back to the hostname slug and leaving `writeMachineKey`'s + * chosen key unprovisioned). Mirrored verbatim in + * packages/rt-client/src/settings/paths.ts — the two must agree or a + * machine-key value could pass one side's check and fail the other's. + */ +export function isSafeMachineKeySegment(v: string): boolean { + return v.length > 0 && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\"); +} + // ─── Tray app (MAT-383) ─────────────────────────────────────────────────────── // // The single source of truth for the tray app's on-disk names/paths, shared diff --git a/packages/rt-client/src/settings/paths.ts b/packages/rt-client/src/settings/paths.ts index d42044e7..43cf2a90 100644 --- a/packages/rt-client/src/settings/paths.ts +++ b/packages/rt-client/src/settings/paths.ts @@ -29,7 +29,7 @@ export function teamSettingsPath(team: string): string { /** * ~/.mattstack/user/local//settings.local.jsonc — the machine - * store (path literals legal here only). + * store, TRACKED and keyed per machine (path literals legal here only). */ export function machineSettingsPath(): string { return join(home(), ".mattstack", "user", "local", machineKey(), "settings.local.jsonc"); @@ -62,7 +62,7 @@ export function machineKey(): string { const override = join(home(), ".mattstack", "machine-key"); try { const v = readFileSync(override, "utf8").trim(); - if (v && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\")) return v; + if (isSafeMachineKeySegment(v)) return v; } catch { // no override file — fall through to the hostname slug } @@ -73,3 +73,8 @@ export function machineKey(): string { .replace(/^-+|-+$/g, ""); return slug || "default"; } + +/** Mirrored verbatim from lib/rt-paths.ts's isSafeMachineKeySegment — the two must agree or a machine-key value could pass one side's check and fail the other's. */ +export function isSafeMachineKeySegment(v: string): boolean { + return v.length > 0 && v !== "." && v !== ".." && !v.includes("/") && !v.includes("\\"); +} From 99345e66b2a8e64a42b4b0a8dd8f9e0ac4b72819 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 13:33:40 -0500 Subject: [PATCH 6/9] RT: machine-store docblock and guard test state the constraint, not the review history Co-Authored-By: Claude Fable 5 --- lib/home/__tests__/init-exec.test.ts | 5 ++--- lib/rt-paths.ts | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/home/__tests__/init-exec.test.ts b/lib/home/__tests__/init-exec.test.ts index 42d66bb1..46f75d4c 100644 --- a/lib/home/__tests__/init-exec.test.ts +++ b/lib/home/__tests__/init-exec.test.ts @@ -273,9 +273,8 @@ describe("createRealExecSeam", () => { } }); - // The reviewer proved the clobber live: writeSymlink alone unconditionally - // unlinked a real skills.jsonc. isRealFile is the guard runStep checks - // FIRST — this proves it against a genuine real file, not a fake. + // isRealFile is the guard runStep checks before any unlink; a genuine file + // (not a fake seam) must trip it, or writeSymlink would clobber user content. test("isRealFile is true for a genuine file, distinguishing it from a symlink", async () => { const home = mkdtempSync(join(tmpdir(), "rt-home-exec-realfile-")); try { diff --git a/lib/rt-paths.ts b/lib/rt-paths.ts index b6f946a5..b8cae7b2 100644 --- a/lib/rt-paths.ts +++ b/lib/rt-paths.ts @@ -95,10 +95,9 @@ export function teamSettingsPath(team: string): string { /** * ~/.mattstack/user/local//settings.local.jsonc — the machine - * store: local overrides, TRACKED and keyed per machine (ruling 2 — machine - * scope travels, it just never collides, since each machine only ever - * writes its own `local//`). Nested per machine so multiple machines - * sharing the synced `user/` tree don't collide on one local-overrides file. + * store: local overrides, TRACKED and keyed per machine. Each machine writes + * only its own `local//`, so machines sharing the synced `user/` tree + * never collide on one local-overrides file. */ export function machineSettingsPath(): string { return join(home(), ".mattstack", "user", "local", machineKey(), "settings.local.jsonc"); From 8e8704c6c8d10ad9d5870a1585099fdc03090aff Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 13:51:01 -0500 Subject: [PATCH 7/9] RT: e2e settings/endpoint fixtures seed stores at the reroot layout settings.test.ts and endpoint.test.ts still seeded the pre-reroot store paths (user/settings.jsonc, teams//mattstack/settings.jsonc, .mattstack/settings.local.jsonc). Rebuild the fixture paths through lib/rt-paths.ts's own constructors, pinning machineKey() via the machine-key override file so machineSettingsPath() is deterministic across hosts. Co-Authored-By: Claude Fable 5 --- e2e/tests/endpoint.test.ts | 11 ++++++++++- e2e/tests/settings.test.ts | 24 ++++++++++++++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/e2e/tests/endpoint.test.ts b/e2e/tests/endpoint.test.ts index 1d3c8c7d..887085cb 100644 --- a/e2e/tests/endpoint.test.ts +++ b/e2e/tests/endpoint.test.ts @@ -27,6 +27,7 @@ import { execFileSync } from "child_process"; import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { join } from "path"; import { createTestHome, RT_BINARY } from "../harness.ts"; +import { machineSettingsPath } from "../../lib/rt-paths.ts"; // ─── Shared helpers (mirroring e2e/tests/events.test.ts) ───────────────────── @@ -236,8 +237,16 @@ describe("rt endpoint / intercept (just-works e2e)", () => { // settings resolver instead of the (now-gone) per-repo config.json). const identity = "github.com/rt-test/endpoint-repo"; mkdirSync(join(home, ".mattstack"), { recursive: true }); + // Pin machineKey() before computing machineSettingsPath() — hostname + // slugs vary per CI host, and this test's path must be deterministic. + writeFileSync(join(home, ".mattstack", "machine-key"), "e2e-endpoint-machine"); + const outerHome = process.env.HOME; + process.env.HOME = home; + const machineStorePath = machineSettingsPath(); + process.env.HOME = outerHome; + mkdirSync(join(machineStorePath, ".."), { recursive: true }); writeFileSync( - join(home, ".mattstack", "settings.local.jsonc"), + machineStorePath, JSON.stringify( { repos: { diff --git a/e2e/tests/settings.test.ts b/e2e/tests/settings.test.ts index 57dc91db..bfa303aa 100644 --- a/e2e/tests/settings.test.ts +++ b/e2e/tests/settings.test.ts @@ -30,6 +30,7 @@ import { execFileSync } from "child_process"; import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, utimesSync, writeFileSync } from "fs"; import { join } from "path"; import { createTestHome, RT_BINARY } from "../harness.ts"; +import { machineSettingsPath, teamSettingsPath, userSettingsPath } from "../../lib/rt-paths.ts"; // ─── Shared helpers (mirroring e2e/tests/endpoint.test.ts) ─────────────────── @@ -99,8 +100,10 @@ const REPO_NAME = "settings-repo"; const REMOTE_URL = "git@github.com:rt-test/settings-repo.git"; const IDENTITY = "github.com/rt-test/settings-repo"; const TEAM = "e2eteam"; +/** Pinned via the `~/.mattstack/machine-key` override so machineSettingsPath() is deterministic across CI hosts. */ +const MACHINE_KEY = "e2e-settings-machine"; -/** Store paths inside the test HOME (mirroring lib/rt-paths.ts). */ +/** Store paths inside the test HOME, built with lib/rt-paths.ts's own constructors. */ let userStore = ""; let teamStore = ""; let machineStore = ""; @@ -293,9 +296,22 @@ describe("rt settings (four stores, one resolver — e2e)", () => { mkdirSync(join(rtDir, "repos", REPO_NAME), { recursive: true }); writeFileSync(join(rtDir, "repos.json"), JSON.stringify({ [REPO_NAME]: repoPath }, null, 2)); - userStore = join(home, ".mattstack", "user", "settings.jsonc"); - teamStore = join(home, ".mattstack", "teams", TEAM, "mattstack", "settings.jsonc"); - machineStore = join(home, ".mattstack", "settings.local.jsonc"); + // Pin machineKey() before computing machineSettingsPath() — hostname + // slugs vary per CI host, and this test's paths must be deterministic. + mkdirSync(join(home, ".mattstack"), { recursive: true }); + writeFileSync(join(home, ".mattstack", "machine-key"), MACHINE_KEY); + + // rt-paths.ts resolves HOME at call time, so swap this (outer) process's + // HOME briefly to compute the fixture's paths through the same + // constructors the daemon subprocess uses, rather than re-deriving the + // layout as literals here. + const outerHome = process.env.HOME; + process.env.HOME = home; + userStore = userSettingsPath(); + teamStore = teamSettingsPath(TEAM); + machineStore = machineSettingsPath(); + process.env.HOME = outerHome; + // The ZONE ROOT, not the mattstack/ dir the settings file lives in. hookStub = join(home, ".mattstack", "teams", TEAM, "hook.sh"); From ff48f211dee10fe5be8a3c422bfa7060ad3efc4c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 14:13:12 -0500 Subject: [PATCH 8/9] RT: e2e settings fixtures pin the on-disk layout, HOME swap is exception-safe The store-path assertions compared the binary's reported provenance to the same rt-paths.ts constructors used to seed the fixture, so a layout regression there would move both sides together and stay green. Add literal expect()s in beforeAll pinning user/settings.user.jsonc, teams//mattstack/settings.team.jsonc, and user/local//settings.local.jsonc against the constructor output. Wrap the outer-process HOME swap (needed because the bunfig preload already repoints this process's HOME to its own temp dir, so the constructors must be pointed at the FIXTURE's temp dir instead) in try/finally, restoring with delete when the prior value was unset. Co-Authored-By: Claude Fable 5 --- e2e/tests/endpoint.test.ts | 25 ++++++++++++++++++---- e2e/tests/settings.test.ts | 43 +++++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/e2e/tests/endpoint.test.ts b/e2e/tests/endpoint.test.ts index 887085cb..bd347afa 100644 --- a/e2e/tests/endpoint.test.ts +++ b/e2e/tests/endpoint.test.ts @@ -48,6 +48,26 @@ function freePort(): number { return port; } +/** + * The bunfig preload (test-setup.ts) already repointed THIS process's HOME to + * its own throwaway temp dir before any module loaded, so machineSettingsPath() + * needs a further, temporary swap to `fakeHome` to compute the path for the + * fixture under test rather than that preload dir. try/finally so a throwing + * constructor can't leave later tests in this same process running against + * the fixture's HOME; `delete` (not `= undefined`) because an unset + * `outerHome` must not stringify back in as `"undefined"`. + */ +function withHome(fakeHome: string, fn: () => T): T { + const outerHome = process.env.HOME; + process.env.HOME = fakeHome; + try { + return fn(); + } finally { + if (outerHome === undefined) delete process.env.HOME; + else process.env.HOME = outerHome; + } +} + /** Bind-probe, same shape as the allocator's real `canBind`. */ function canBind(port: number): boolean { try { @@ -240,10 +260,7 @@ describe("rt endpoint / intercept (just-works e2e)", () => { // Pin machineKey() before computing machineSettingsPath() — hostname // slugs vary per CI host, and this test's path must be deterministic. writeFileSync(join(home, ".mattstack", "machine-key"), "e2e-endpoint-machine"); - const outerHome = process.env.HOME; - process.env.HOME = home; - const machineStorePath = machineSettingsPath(); - process.env.HOME = outerHome; + const machineStorePath = withHome(home, () => machineSettingsPath()); mkdirSync(join(machineStorePath, ".."), { recursive: true }); writeFileSync( machineStorePath, diff --git a/e2e/tests/settings.test.ts b/e2e/tests/settings.test.ts index bfa303aa..921c8e66 100644 --- a/e2e/tests/settings.test.ts +++ b/e2e/tests/settings.test.ts @@ -51,6 +51,26 @@ function freePort(): number { return port; } +/** + * The bunfig preload (test-setup.ts) already repointed THIS process's HOME to + * its own throwaway temp dir before any module loaded, so the rt-paths.ts + * constructors need a further, temporary swap to `fakeHome` to compute paths + * for the fixture under test rather than that preload dir. try/finally so a + * throwing constructor can't leave later tests in this same process running + * against the fixture's HOME; `delete` (not `= undefined`) because an unset + * `outerHome` must not stringify back in as `"undefined"`. + */ +function withHome(fakeHome: string, fn: () => T): T { + const outerHome = process.env.HOME; + process.env.HOME = fakeHome; + try { + return fn(); + } finally { + if (outerHome === undefined) delete process.env.HOME; + else process.env.HOME = outerHome; + } +} + /** Bind-probe, same shape as the allocator's real `canBind`. */ function canBind(port: number): boolean { try { @@ -301,16 +321,19 @@ describe("rt settings (four stores, one resolver — e2e)", () => { mkdirSync(join(home, ".mattstack"), { recursive: true }); writeFileSync(join(home, ".mattstack", "machine-key"), MACHINE_KEY); - // rt-paths.ts resolves HOME at call time, so swap this (outer) process's - // HOME briefly to compute the fixture's paths through the same - // constructors the daemon subprocess uses, rather than re-deriving the - // layout as literals here. - const outerHome = process.env.HOME; - process.env.HOME = home; - userStore = userSettingsPath(); - teamStore = teamSettingsPath(TEAM); - machineStore = machineSettingsPath(); - process.env.HOME = outerHome; + withHome(home, () => { + userStore = userSettingsPath(); + teamStore = teamSettingsPath(TEAM); + machineStore = machineSettingsPath(); + }); + + // Pin the on-disk SHAPE too, not just internal agreement with the + // constructors — a layout regression inside rt-paths.ts would move the + // constructor output and this assertion's expectation together and the + // suite would stay green without these literals. + expect(userStore).toBe(join(home, ".mattstack", "user", "settings.user.jsonc")); + expect(teamStore).toBe(join(home, ".mattstack", "teams", TEAM, "mattstack", "settings.team.jsonc")); + expect(machineStore).toBe(join(home, ".mattstack", "user", "local", MACHINE_KEY, "settings.local.jsonc")); // The ZONE ROOT, not the mattstack/ dir the settings file lives in. hookStub = join(home, ".mattstack", "teams", TEAM, "hook.sh"); From 96eceed454b4b4d831c11ffd3617453e65885e96 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 14:42:16 -0500 Subject: [PATCH 9/9] =?UTF-8?q?RT:=20final=20review=20fixes=20=E2=80=94=20?= =?UTF-8?q?sops=20hint=20cwd,=20mint-vs-clone=20recipient=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 1: the .sops.yaml commit hint printed `git -C add user/.sops.yaml` — ~/.mattstack is no longer a repo on the re-rooted layout, so that command fails on every fresh-machine path. Pins cwd to /user instead, matching where .sops.yaml actually lives. Fix 2: a fresh machine with an empty keychain could mint a brand-new age key, then silently rewrite a freshly-cloned user/.sops.yaml from the OLD recipient (what the cloned user/secrets/*.json are actually encrypted to) to the new one — orphaning those secrets here and, once committed, breaking every other machine still holding the real key. ensureAgeKey now reports whether it minted; ensureHomeAgeKey refuses (leaves the file untouched, surfaces a directed error pointing at `rt home key import`) exactly when a JUST-MINTED key meets an existing .sops.yaml naming a DIFFERENT recipient. A rotation on a machine that already held the right key is unchanged — that's a deliberate rotation, not a fresh machine guessing. Minor: corrects two "no git fallback"/"never committed" comments in rt-client's write.ts and rt-paths.ts-adjacent code now that all three settings stores are tracked repos; extends the "local only until you commit and push" write reminder from team-only to every scope, since nothing auto-commits any of them yet (H2 unbuilt); renames ExecSeam.isRealFile to blocksSymlink (it also correctly blocks on a directory, so the old name undersold what it checks) and adds a test proving the directory case; e2e/tests/endpoint.test.ts's machine-store fixture now mkdirs via dirname() instead of join(path, ".."), matching the rest of the codebase; command-tree-def.ts's `rt home init` description no longer implies clone happens before the plan is printed. rt-client dist rebuilt for the write.ts change. --- commands/__tests__/home.test.ts | 77 +++++++++++++++++-- commands/home.ts | 45 +++++++++-- e2e/tests/endpoint.test.ts | 4 +- lib/command-tree-def.ts | 2 +- lib/home/__tests__/age-key.test.ts | 4 +- lib/home/__tests__/init-exec.test.ts | 47 +++++++---- lib/home/age-key.ts | 13 +++- lib/home/init-exec.ts | 15 ++-- .../src/settings/__tests__/write.test.ts | 24 +++++- packages/rt-client/src/settings/write.ts | 29 +++---- 10 files changed, 205 insertions(+), 55 deletions(-) diff --git a/commands/__tests__/home.test.ts b/commands/__tests__/home.test.ts index f4994407..59df3e7c 100644 --- a/commands/__tests__/home.test.ts +++ b/commands/__tests__/home.test.ts @@ -30,7 +30,7 @@ class FakeSopsYamlSeam implements SopsYamlSeam { } } -/** No key in the keychain yet; ensureAgeKey mints one — never touches the real keychain. */ +/** No key in the keychain yet; ensureAgeKey MINTS one — never touches the real keychain. */ class FakeAgeKeySeam implements AgeKeySeam { calls: string[][] = []; @@ -47,6 +47,22 @@ class FakeAgeKeySeam implements AgeKeySeam { } } +/** A key ALREADY exists in the keychain; ensureAgeKey only DERIVES its public half, never mints — never touches the real keychain. */ +class FakeAgeKeySeamWithExistingKey implements AgeKeySeam { + calls: string[][] = []; + + async run(cmd: string[]): Promise { + this.calls.push(cmd); + if (cmd[1] === "find-generic-password") { + return { code: 0, stdout: `${FAKE_PRIVATE_KEY}\n`, stderr: "" }; + } + if (cmd[0] === "age-keygen" && cmd[1] === "-y") { + return { code: 0, stdout: `${FAKE_PUBLIC_KEY}\n`, stderr: "" }; + } + throw new Error(`FakeAgeKeySeamWithExistingKey: unexpected call ${cmd.join(" ")}`); + } +} + function fakeProbes(overrides: Partial): HomeProbes { return { isGitRepo: () => false, @@ -134,8 +150,8 @@ class FakeSeam implements ExecSeam { this.calls.push({ kind: "exists", arg: path }); return false; } - async isRealFile(path: string): Promise { - this.calls.push({ kind: "isRealFile", arg: path }); + async blocksSymlink(path: string): Promise { + this.calls.push({ kind: "blocksSymlink", arg: path }); return false; } async writeSymlink(path: string, target: string): Promise { @@ -207,9 +223,9 @@ describe("homeInit", () => { expect(sopsYamlSeam.writes).toEqual([]); }); - test("fully provisioned: an existing .sops.yaml with a stale recipient (key rotation) is rewritten", async () => { + test("fully provisioned, key ALREADY in the keychain (not minted): a stale .sops.yaml recipient is rewritten — a deliberate rotation, not a fresh machine guessing", async () => { const seam = new FakeSeam(); - const ageKeySeam = new FakeAgeKeySeam(); + const ageKeySeam = new FakeAgeKeySeamWithExistingKey(); const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml("age1stale") }); await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); @@ -217,6 +233,57 @@ describe("homeInit", () => { expect(sopsYamlSeam.files.get(SOPS_YAML_PATH)).toBe(renderSopsYaml(FAKE_PUBLIC_KEY)); }); + test("fully provisioned, key ALREADY in the keychain, recipient matches: no-op", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeamWithExistingKey(); + const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml(FAKE_PUBLIC_KEY) }); + + const { exitCode } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); + + expect(exitCode).toBeUndefined(); + expect(sopsYamlSeam.writes).toEqual([]); + }); + + test("fully provisioned, key JUST MINTED (fresh/empty keychain), a cloned .sops.yaml names a DIFFERENT recipient: refuses, leaves the file untouched, exits 1", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); // empty keychain -> mints + const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml("age1the-other-machines-recipient") }); + + const { exitCode, errors } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); + + expect(exitCode).toBe(1); + expect(sopsYamlSeam.writes).toEqual([]); + expect(sopsYamlSeam.files.get(SOPS_YAML_PATH)).toBe(renderSopsYaml("age1the-other-machines-recipient")); + expect(errors.some((e) => e.includes("age1the-other-machines-recipient"))).toBe(true); + expect(errors.some((e) => e.includes("rt home key import"))).toBe(true); + }); + + test("fully provisioned, key JUST MINTED, a cloned .sops.yaml already names the SAME recipient: no-op (the astronomically unlikely match is still safe)", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + const sopsYamlSeam = new FakeSopsYamlSeam({ path: SOPS_YAML_PATH, content: renderSopsYaml(FAKE_PUBLIC_KEY) }); + + const { exitCode } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); + + expect(exitCode).toBeUndefined(); + expect(sopsYamlSeam.writes).toEqual([]); + }); + + test("the printed .sops.yaml commit hint pins cwd to user/, not the (no-longer-a-repo) root", async () => { + const seam = new FakeSeam(); + const ageKeySeam = new FakeAgeKeySeam(); + const sopsYamlSeam = new FakeSopsYamlSeam(); + + const { logs } = await runHomeInit(FULLY_PROVISIONED_PROBES(), seam, ageKeySeam, [], sopsYamlSeam); + + const userDir = join(mattstackHome(), "user"); + const hint = logs.find((l) => l.includes("git -C")); + expect(hint).toBeDefined(); + expect(hint).toContain(`git -C ${userDir} add .sops.yaml`); + expect(hint).toContain(`git -C ${userDir} commit -m "home: sops recipient"`); + expect(hint).not.toContain(`git -C ${mattstackHome()} add`); + }); + test("fully provisioned --dry-run: never touches the age key either", async () => { const seam = new FakeSeam(); const ageKeySeam = new FakeAgeKeySeam(); diff --git a/commands/home.ts b/commands/home.ts index fe1d38b1..9b9159bd 100644 --- a/commands/home.ts +++ b/commands/home.ts @@ -124,6 +124,8 @@ function parseUrlArg(args: string[]): string { return value; } +export type EnsureHomeAgeKeyResult = { ok: true } | { ok: false; message: string }; + /** * The sole mint site: `key export` (lib/home/age-key.ts:keyExport) refuses * to mint, precisely so a keychain-access error there can never be mistaken @@ -137,23 +139,48 @@ function parseUrlArg(args: string[]): string { * right recipient is left untouched. `.sops.yaml` is a TRACKED file, so a * write here needs a human commit — the snapshot daemon doesn't exist yet. * + * EXCEPT when this call's key was JUST MINTED (readAgeKey found the + * keychain provably empty) and an existing `.sops.yaml` already names a + * DIFFERENT recipient: that recipient is what the just-cloned `user/secrets/*.json` + * were actually encrypted to, on some other machine. Rewriting here would + * silently orphan them (undecryptable on this machine) and, once committed, + * break every other machine still holding the real key — so this refuses + * instead, leaving the file untouched. A rotation on a machine that ALREADY + * held the right key (not minted) is unchanged: that's a deliberate rotation, + * not a fresh machine guessing. + * * Called only after the init plan (which clones user/ when it's missing) * has run to completion, so user/ always already exists by the time this * writes into it. */ -async function ensureHomeAgeKey(seams: AgeKeySeam, sopsYamlSeam: SopsYamlSeam = defaultSopsYamlSeam()): Promise { - const { publicKey } = await ensureAgeKey(seams); +async function ensureHomeAgeKey( + seams: AgeKeySeam, + sopsYamlSeam: SopsYamlSeam = defaultSopsYamlSeam(), +): Promise { + const { publicKey, minted } = await ensureAgeKey(seams); // Lives under user/ (not the repo root): sops matches path_regex cwd-relative // and every sops spawn pins cwd to /user (store.ts), so // .sops.yaml must sit there too for that discovery to find it. - const sopsYamlPath = join(mattstackHome(), "user", ".sops.yaml"); + const userDir = join(mattstackHome(), "user"); + const sopsYamlPath = join(userDir, ".sops.yaml"); const existing = sopsYamlSeam.read(sopsYamlPath); - if (existing === null || sopsYamlRecipient(existing) !== publicKey) { + const existingRecipient = existing === null ? null : sopsYamlRecipient(existing); + + if (minted && existing !== null && existingRecipient !== publicKey) { + return { + ok: false, + message: + `secrets are encrypted to ${existingRecipient ?? "an unrecognized recipient"}; ` + + "import the age key from your password manager (`rt home key import`) before initializing.", + }; + } + + if (existing === null || existingRecipient !== publicKey) { sopsYamlSeam.write(sopsYamlPath, renderSopsYaml(publicKey)); console.log( `rt home init: wrote ${sopsYamlPath} (recipient ${publicKey}) — it's tracked, so commit it:\n` + - ` git -C ${mattstackHome()} add user/.sops.yaml && git -C ${mattstackHome()} commit -m "home: sops recipient"`, + ` git -C ${userDir} add .sops.yaml && git -C ${userDir} commit -m "home: sops recipient"`, ); } @@ -161,6 +188,8 @@ async function ensureHomeAgeKey(seams: AgeKeySeam, sopsYamlSeam: SopsYamlSeam = `rt home init: age key ready — recipient ${publicKey}.\n` + " Run `rt home key export` to save the private key to your password manager.", ); + + return { ok: true }; } export async function homeInit( @@ -228,7 +257,11 @@ export async function homeInit( // Mint (or backfill) BEFORE the success line: printing success ahead of a // failed mint would tell the operator init worked while `rt secrets set` // still has no key or creation rule to encrypt against. - await ensureHomeAgeKey(ageKeySeam, sopsYamlSeam); + const ageKeyResult = await ensureHomeAgeKey(ageKeySeam, sopsYamlSeam); + if (!ageKeyResult.ok) { + console.error(`\nrt home init: ${ageKeyResult.message}`); + process.exit(1); + } if (plan.blocked === "skills-symlink-real-file") { console.error(`\nrt home init: provisioning finished, but the skills.jsonc symlink is still blocked — see above.`); diff --git a/e2e/tests/endpoint.test.ts b/e2e/tests/endpoint.test.ts index bd347afa..b6e2d67a 100644 --- a/e2e/tests/endpoint.test.ts +++ b/e2e/tests/endpoint.test.ts @@ -25,7 +25,7 @@ import { describe, test, expect, beforeAll, afterAll } from "bun:test"; import { execFileSync } from "child_process"; import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; -import { join } from "path"; +import { dirname, join } from "path"; import { createTestHome, RT_BINARY } from "../harness.ts"; import { machineSettingsPath } from "../../lib/rt-paths.ts"; @@ -261,7 +261,7 @@ describe("rt endpoint / intercept (just-works e2e)", () => { // slugs vary per CI host, and this test's path must be deterministic. writeFileSync(join(home, ".mattstack", "machine-key"), "e2e-endpoint-machine"); const machineStorePath = withHome(home, () => machineSettingsPath()); - mkdirSync(join(machineStorePath, ".."), { recursive: true }); + mkdirSync(dirname(machineStorePath), { recursive: true }); writeFileSync( machineStorePath, JSON.stringify( diff --git a/lib/command-tree-def.ts b/lib/command-tree-def.ts index 22674020..a017336a 100644 --- a/lib/command-tree-def.ts +++ b/lib/command-tree-def.ts @@ -695,7 +695,7 @@ export const TREE: Record = { description: "The git-backed ~/.mattstack/user personal repo", subcommands: { init: { - description: "Provision this machine: clone the user repo, then print and run the provisioning plan", + description: "Provision this machine: print, then run, the plan (which clones the user repo as one of its steps)", module: "./commands/home.ts", fn: "homeInit", args: [ diff --git a/lib/home/__tests__/age-key.test.ts b/lib/home/__tests__/age-key.test.ts index 3adee3f9..c76f881c 100644 --- a/lib/home/__tests__/age-key.test.ts +++ b/lib/home/__tests__/age-key.test.ts @@ -113,7 +113,7 @@ describe("ensureAgeKey", () => { const result = await ensureAgeKey(seam); - expect(result).toEqual({ publicKey: FAKE_PUBLIC_KEY }); + expect(result).toEqual({ publicKey: FAKE_PUBLIC_KEY, minted: true }); expect(seam.calls.map((c) => c.cmd)).toEqual([FIND_CMD, ["age-keygen"], [...ADD_CMD_PREFIX, FAKE_PRIVATE_KEY]]); }); @@ -136,7 +136,7 @@ describe("ensureAgeKey", () => { const result = await ensureAgeKey(seam); - expect(result).toEqual({ publicKey: FAKE_PUBLIC_KEY }); + expect(result).toEqual({ publicKey: FAKE_PUBLIC_KEY, minted: false }); expect(seam.calls.map((c) => c.cmd)).toEqual([FIND_CMD, ["age-keygen", "-y"]]); // The private key is piped via stdin, never argv. const deriveCall = seam.calls.find((c) => c.cmd[0] === "age-keygen" && c.cmd[1] === "-y"); diff --git a/lib/home/__tests__/init-exec.test.ts b/lib/home/__tests__/init-exec.test.ts index 46f75d4c..996b5341 100644 --- a/lib/home/__tests__/init-exec.test.ts +++ b/lib/home/__tests__/init-exec.test.ts @@ -12,7 +12,7 @@ type RecordedCall = | { kind: "writeFile"; path: string; content: string } | { kind: "mkdirp"; path: string } | { kind: "exists"; path: string } - | { kind: "isRealFile"; path: string } + | { kind: "blocksSymlink"; path: string } | { kind: "writeSymlink"; path: string; target: string }; const noopLog = () => {}; @@ -25,7 +25,7 @@ class FakeExecSeam implements ExecSeam { private opts: { failRun?: (cmd: string[]) => string | undefined; exists?: (path: string) => boolean; - isRealFile?: (path: string) => boolean; + blocksSymlink?: (path: string) => boolean; } = {}, ) {} @@ -49,9 +49,9 @@ class FakeExecSeam implements ExecSeam { return this.opts.exists?.(path) ?? false; } - async isRealFile(path: string): Promise { - this.calls.push({ kind: "isRealFile", path }); - return this.opts.isRealFile?.(path) ?? false; + async blocksSymlink(path: string): Promise { + this.calls.push({ kind: "blocksSymlink", path }); + return this.opts.blocksSymlink?.(path) ?? false; } async writeSymlink(path: string, target: string): Promise { @@ -155,20 +155,20 @@ describe("executeInitPlan", () => { describe("writeSkillsSymlink — re-checked at exec time, never trusts the plan-build-time probe", () => { test("nothing (or a symlink) at the root path: links skills.jsonc -> user/skills.jsonc", async () => { - const seam = new FakeExecSeam({ isRealFile: () => false }); + const seam = new FakeExecSeam({ blocksSymlink: () => false }); const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; const result = await executeInitPlan(steps, seam, noopLog); expect(result).toEqual({ ok: true }); expect(seam.calls).toEqual([ - { kind: "isRealFile", path: "skills.jsonc" }, + { kind: "blocksSymlink", path: "skills.jsonc" }, { kind: "writeSymlink", path: "skills.jsonc", target: join("user", "skills.jsonc") }, ]); }); test("a REAL file at the root path: the step fails, and writeSymlink (so unlink) is never called", async () => { - const seam = new FakeExecSeam({ isRealFile: () => true }); + const seam = new FakeExecSeam({ blocksSymlink: () => true }); const steps: InitStep[] = [{ kind: "writeSkillsSymlink" }]; const result = await executeInitPlan(steps, seam, noopLog); @@ -178,13 +178,13 @@ describe("executeInitPlan", () => { expect(result.failedStep).toBe("writeSkillsSymlink"); expect(result.stderr).toContain("refusing to overwrite"); } - expect(seam.calls).toEqual([{ kind: "isRealFile", path: "skills.jsonc" }]); + expect(seam.calls).toEqual([{ kind: "blocksSymlink", path: "skills.jsonc" }]); expect(seam.calls.some((c) => c.kind === "writeSymlink")).toBe(false); }); }); test("runs a full fresh-machine plan's steps in order", async () => { - const seam = new FakeExecSeam({ exists: () => false, isRealFile: () => false }); + const seam = new FakeExecSeam({ exists: () => false, blocksSymlink: () => false }); const steps = buildInitPlan( { userRepoPresent: false, @@ -215,7 +215,7 @@ describe("executeInitPlan", () => { "writeFile", // user/snapshot-owners.jsonc "writeFile", // machine-key "mkdirp", // user/local/mbp-14 - "isRealFile", // skills.jsonc + "blocksSymlink", // skills.jsonc "writeSymlink", // skills.jsonc ]); }); @@ -258,12 +258,12 @@ describe("createRealExecSeam", () => { expect(readFileSync(join(home, "user", "skills.jsonc"), "utf8")).toBe("{}\n"); expect(await seam.exists("user/skills.jsonc")).toBe(true); - expect(await seam.isRealFile("skills.jsonc")).toBe(false); // absent + expect(await seam.blocksSymlink("skills.jsonc")).toBe(false); // absent await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); const st = lstatSync(join(home, "skills.jsonc")); expect(st.isSymbolicLink()).toBe(true); expect(readlinkSync(join(home, "skills.jsonc"))).toBe(join("user", "skills.jsonc")); - expect(await seam.isRealFile("skills.jsonc")).toBe(false); // a symlink, not a real file + expect(await seam.blocksSymlink("skills.jsonc")).toBe(false); // a symlink, not a real file // writeSymlink replaces whatever was already there. await seam.writeSymlink("skills.jsonc", join("user", "skills.jsonc")); @@ -273,15 +273,30 @@ describe("createRealExecSeam", () => { } }); - // isRealFile is the guard runStep checks before any unlink; a genuine file + // blocksSymlink is the guard runStep checks before any unlink; a genuine file // (not a fake seam) must trip it, or writeSymlink would clobber user content. - test("isRealFile is true for a genuine file, distinguishing it from a symlink", async () => { + test("blocksSymlink is true for a genuine file, distinguishing it from a symlink", async () => { const home = mkdtempSync(join(tmpdir(), "rt-home-exec-realfile-")); try { const seam = createRealExecSeam(home); writeFileSync(join(home, "skills.jsonc"), '{"real": "content"}\n'); - expect(await seam.isRealFile("skills.jsonc")).toBe(true); + expect(await seam.blocksSymlink("skills.jsonc")).toBe(true); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + // A directory at the root path is exactly as unsafe to unlink+symlink over + // as a real file — the name says "blocks a symlink write", not "is a file", + // so this must read true too. + test("blocksSymlink is true for a directory, not just a plain file", async () => { + const home = mkdtempSync(join(tmpdir(), "rt-home-exec-realdir-")); + try { + const seam = createRealExecSeam(home); + mkdirSync(join(home, "skills.jsonc")); + + expect(await seam.blocksSymlink("skills.jsonc")).toBe(true); } finally { rmSync(home, { recursive: true, force: true }); } diff --git a/lib/home/age-key.ts b/lib/home/age-key.ts index 630ac9d9..a20917ba 100644 --- a/lib/home/age-key.ts +++ b/lib/home/age-key.ts @@ -99,15 +99,22 @@ function parseAgeKeygenOutput(output: string): { publicKey: string; privateKey: * on top of that: if an item exists anyway, the keychain write itself * fails rather than overwriting, the last-resort guard against clobbering * the custodied key. + * + * `minted` tells the caller whether THIS call is the one that generated the + * key (vs. deriving the public half of one already in the keychain) — the + * signal `rt home init` needs to tell "a machine with no key yet" apart + * from "a machine that already has the right key", since only the former + * can safely treat a mismatched `.sops.yaml` recipient as stale rather than + * as evidence of secrets encrypted to a key this machine doesn't hold. */ -export async function ensureAgeKey(seams: AgeKeySeam): Promise<{ publicKey: string }> { +export async function ensureAgeKey(seams: AgeKeySeam): Promise<{ publicKey: string; minted: boolean }> { const existing = await readAgeKey(seams); if ("key" in existing) { const derived = await seams.run(["age-keygen", "-y"], { input: existing.key, sensitive: true }); if (derived.code !== 0) { throw new Error(`age-keygen -y: could not derive the public key from the stored private key\n${derived.stderr}`); } - return { publicKey: derived.stdout.trim() }; + return { publicKey: derived.stdout.trim(), minted: false }; } const generated = await seams.run(["age-keygen"], { sensitive: true }); @@ -121,7 +128,7 @@ export async function ensureAgeKey(seams: AgeKeySeam): Promise<{ publicKey: stri throw new Error(`security add-generic-password: failed to store the age key in the keychain\n${stored.stderr}`); } - return { publicKey }; + return { publicKey, minted: true }; } export function renderSopsYaml(publicKey: string): string { diff --git a/lib/home/init-exec.ts b/lib/home/init-exec.ts index 6d1db061..72afe095 100644 --- a/lib/home/init-exec.ts +++ b/lib/home/init-exec.ts @@ -23,9 +23,14 @@ export interface ExecSeam { mkdirp(path: string): Promise; /** Any entry at all (file, dir, or symlink) — used to seed tracked files only into a genuinely empty clone. */ exists(path: string): Promise; - /** True only for a REAL (non-symlink) entry — absent or a symlink both read false. */ - isRealFile(path: string): Promise; - /** Idempotent: replaces whatever (if anything) already sits at `path`. Callers must confirm via isRealFile first — this never itself refuses a real file. */ + /** + * True for any entry that a symlink write must not clobber: a real file + * OR a directory — both would be silently destroyed by unlink+symlink. + * False for absent or an existing symlink (writeSymlink safely replaces + * either of those). + */ + blocksSymlink(path: string): Promise; + /** Idempotent: replaces whatever (if anything) already sits at `path`. Callers must confirm via blocksSymlink first — this never itself refuses a real file or directory. */ writeSymlink(path: string, target: string): Promise; } @@ -92,7 +97,7 @@ async function runStep(step: InitStep, exec: ExecSeam, log: StepLog): Promise user/skills.jsonc"); @@ -143,7 +148,7 @@ export function createRealExecSeam(home: string): ExecSeam { async exists(path) { return existsSync(join(home, path)); }, - async isRealFile(path) { + async blocksSymlink(path) { try { return !lstatSync(join(home, path)).isSymbolicLink(); } catch { diff --git a/packages/rt-client/src/settings/__tests__/write.test.ts b/packages/rt-client/src/settings/__tests__/write.test.ts index 6b68c6c4..cc872f49 100644 --- a/packages/rt-client/src/settings/__tests__/write.test.ts +++ b/packages/rt-client/src/settings/__tests__/write.test.ts @@ -258,7 +258,11 @@ describe("settings/write", () => { expect(stderrWrites.some((line) => /commit|push/i.test(line))).toBe(true); }); - test("a user-scope write prints no reminder", () => { + // Every scope is a tracked repo with nothing auto-committing a write + // (H2, the snapshot daemon, is unbuilt) — user and machine writes get + // the same local-only reminder team writes always have, naming their + // own store path. + test("a user-scope write also prints the local-only reminder, naming the user store", () => { const stderrWrites: string[] = []; const orig = console.error; console.error = (...args: unknown[]) => { @@ -269,7 +273,23 @@ describe("settings/write", () => { } finally { console.error = orig; } - expect(stderrWrites.length).toBe(0); + expect(stderrWrites.some((line) => /commit|push/i.test(line))).toBe(true); + expect(stderrWrites.some((line) => line.includes(userSettingsPath()))).toBe(true); + }); + + test("a machine-scope write also prints the local-only reminder, naming the machine store", () => { + const stderrWrites: string[] = []; + const orig = console.error; + console.error = (...args: unknown[]) => { + stderrWrites.push(args.map(String).join(" ")); + }; + try { + setSetting("rt.worktrees", { onDeck: 3 }, "machine"); + } finally { + console.error = orig; + } + expect(stderrWrites.some((line) => /commit|push/i.test(line))).toBe(true); + expect(stderrWrites.some((line) => line.includes(machineSettingsPath()))).toBe(true); }); }); diff --git a/packages/rt-client/src/settings/write.ts b/packages/rt-client/src/settings/write.ts index ce331367..14417d40 100644 --- a/packages/rt-client/src/settings/write.ts +++ b/packages/rt-client/src/settings/write.ts @@ -85,11 +85,12 @@ * ── Writes are write-temp-then-rename ─────────────────────────────────── * Mirrors `lib/json-store.ts`'s `writeJson`: the edited text is written to a * `...tmp` file in the SAME directory, then renamed onto - * the real path — stores never tear, matching the rest of rt's persistence - * (the machine store especially has no git fallback to recover a partial - * write from). The tmp file carries the edited TEXT exactly as `applyEdits` - * produced it, never round-tripped through `JSON.stringify` — that's what - * keeps comments alive. + * the real path — stores never tear, matching the rest of rt's persistence. + * All three stores are tracked repos now, but nothing auto-commits a write + * (H2, the snapshot daemon, is unbuilt) — a torn write would sit as a + * corrupt uncommitted file until a human noticed. The tmp file carries the + * edited TEXT exactly as `applyEdits` produced it, never round-tripped + * through `JSON.stringify` — that's what keeps comments alive. */ import { existsSync, mkdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from "fs"; @@ -151,11 +152,12 @@ export function setSetting(key: string, value: unknown, scope: SettingScope, opt writeIntoStore(storePath, jsonPath, value, /* createIfMissing */ scope !== "team"); - if (scope === "team") { - console.error( - `rt: wrote "${key}" to the local team store (${storePath}) — this is local only until you commit and push it.`, - ); - } + // All three stores are tracked repos with nothing auto-committing a write + // (H2, the snapshot daemon, is unbuilt) — every scope gets the reminder, + // not just team. + console.error( + `rt: wrote "${key}" to the local ${scope} store (${storePath}) — this is local only until you commit and push it.`, + ); } function migratedFalseMessage(key: string, def: SettingDef): string { @@ -272,9 +274,10 @@ function writeIntoStore(storePath: string, jsonPath: JSONPath, value: unknown, c const finalText = next.endsWith("\n") ? next : `${next}\n`; // Write-temp-then-rename in the same directory, mirroring - // lib/json-store.ts's writeJson — stores never tear, and the machine store - // especially has no git fallback to recover a partial write from. The - // edited TEXT is written as-is, never round-tripped through + // lib/json-store.ts's writeJson — stores never tear. All three stores are + // tracked repos now, but nothing auto-commits a write (H2 is unbuilt), so + // a torn write would sit as a corrupt uncommitted file until a human + // noticed. The edited TEXT is written as-is, never round-tripped through // JSON.stringify, so comments and formatting survive. const tmp = `${storePath}.${process.pid}.${randomBytes(6).toString("hex")}.tmp`; try {