From 65e0c64d4177449591a2199c4a02055df182009c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 09:08:56 -0500 Subject: [PATCH 1/4] RT: secrets:read gains a deck-scoped whitelist (cfApiToken/cfZoneId) + rt-client dist guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit secrets:read now takes payload.scope ("extension" default | "deck"), each reading its own encrypted domain behind the same token gate — lets deck fetch its CF credentials without widening the extension's existing whitelist. Also fixes the rt-client packaging stall: prepack builds dist/ for npm pack/publish, and a new test rebuilds dist/ and checks it before file: consumers (mr-board, gitq) can silently pick up a stale one. Co-Authored-By: Claude Fable 5 --- lib/daemon/__tests__/secrets-handler.test.ts | 78 +++++++++++++++++++ lib/daemon/handlers/secrets.ts | 55 +++++++++++-- packages/rt-client/package.json | 3 +- packages/rt-client/src/commands.ts | 22 ++++-- .../rt-client/test/dist-freshness.test.ts | 28 +++++++ 5 files changed, 173 insertions(+), 13 deletions(-) create mode 100644 packages/rt-client/test/dist-freshness.test.ts diff --git a/lib/daemon/__tests__/secrets-handler.test.ts b/lib/daemon/__tests__/secrets-handler.test.ts index 134cfe6f..472c1537 100644 --- a/lib/daemon/__tests__/secrets-handler.test.ts +++ b/lib/daemon/__tests__/secrets-handler.test.ts @@ -117,10 +117,12 @@ describe("secrets:forge-token default tracking reader is machine-only", () => { // has no auth of its own — the handler is the only gate there). function readHandler(opts: { extensionSecrets?: () => Promise<{ linearApiKey?: string; gitlabToken?: string }>; + deckSecrets?: () => Promise<{ cfApiToken?: string; cfZoneId?: string }>; apiToken?: string; }) { const h = createSecretsHandlers(fakeCtx, { extensionSecrets: opts.extensionSecrets ?? (async () => ({})), + deckSecrets: opts.deckSecrets ?? (async () => ({})), apiToken: () => opts.apiToken ?? "test-token", }); return h["secrets:read"]; @@ -179,4 +181,80 @@ describe("secrets:read", () => { expect((await h({ token: "shared-secret" })).ok).toBe(true); }); + + test("omitted scope defaults to extension and never touches the deck reader", async () => { + let deckCalled = false; + const h = readHandler({ + extensionSecrets: async () => ({ linearApiKey: "lin_api_x" }), + deckSecrets: async () => { deckCalled = true; return { cfApiToken: "cf-tok" }; }, + }); + + const res = await h({ token: "test-token" }); + + expect(res).toEqual({ ok: true, data: { linearApiKey: "lin_api_x" } }); + expect(deckCalled).toBe(false); + }); +}); + +// scope: "deck" (Task 3) reads a wholly different encrypted domain +// (lib/secrets/store.ts readSecret("deck", ...)) — the whitelist and the +// domain are both per-scope, so an rt-domain value seeded for the extension +// scope must never leak into a deck-scope read, and vice versa. +describe("secrets:read scope", () => { + test("deck scope returns only cfApiToken/cfZoneId, never an rt-domain key seeded for extension scope", async () => { + const h = readHandler({ + extensionSecrets: async () => ({ linearApiKey: "lin_api_x", gitlabToken: "glpat-x" }), + deckSecrets: async () => ({ cfApiToken: "cf-tok", cfZoneId: "zone-1" }), + }); + + const res = await h({ token: "test-token", scope: "deck" }); + + expect(res).toEqual({ ok: true, data: { cfApiToken: "cf-tok", cfZoneId: "zone-1" } }); + expect("linearApiKey" in (res as any).data).toBe(false); + }); + + test("deck scope omits a key entirely when it isn't set", async () => { + const h = readHandler({ deckSecrets: async () => ({ cfApiToken: "cf-tok" }) }); + + const res = await h({ token: "test-token", scope: "deck" }); + + expect(res).toEqual({ ok: true, data: { cfApiToken: "cf-tok" } }); + expect("cfZoneId" in (res as any).data).toBe(false); + }); + + test("extension scope is unchanged when named explicitly", async () => { + const h = readHandler({ extensionSecrets: async () => ({ linearApiKey: "lin_api_x", gitlabToken: "glpat-x" }) }); + + const res = await h({ token: "test-token", scope: "extension" }); + + expect(res).toEqual({ ok: true, data: { linearApiKey: "lin_api_x", gitlabToken: "glpat-x" } }); + }); + + test("bad scope is refused before either reader runs", async () => { + let extensionCalled = false; + let deckCalled = false; + const h = readHandler({ + extensionSecrets: async () => { extensionCalled = true; return {}; }, + deckSecrets: async () => { deckCalled = true; return {}; }, + }); + + const res = await h({ token: "test-token", scope: "bitbucket" as any }); + + expect(res).toEqual({ ok: false, error: "bad-scope" }); + expect(extensionCalled).toBe(false); + expect(deckCalled).toBe(false); + }); + + test("the token gate applies to the deck scope exactly as it does to extension", async () => { + let deckCalled = false; + const h = readHandler({ deckSecrets: async () => { deckCalled = true; return {}; } }); + + const missing = await h({ scope: "deck" }); + expect(missing).toEqual({ ok: false, error: "missing-token" }); + + const wrong = await h({ token: "wrong", scope: "deck" }); + expect(wrong).toEqual({ ok: false, error: "bad-token" }); + + expect(deckCalled).toBe(false); + }); }); diff --git a/lib/daemon/handlers/secrets.ts b/lib/daemon/handlers/secrets.ts index bf23a7db..f4f29441 100644 --- a/lib/daemon/handlers/secrets.ts +++ b/lib/daemon/handlers/secrets.ts @@ -29,14 +29,47 @@ * X-RT-Token header into the payload so HTTP callers (the extension) don't * need to change; a socket caller must read ~/.mattstack/rt/api-token * itself and pass it the same way. + * + * `payload.scope` (Task 3, deck lane) adds a second whitelist alongside the + * extension's, each reading its own encrypted domain — "extension" (default, + * so existing callers need no change) reads the `rt` domain's linearApiKey/ + * gitlabToken; "deck" reads the `deck` domain's cfApiToken/cfZoneId. The + * token gate above applies identically to both; scope is checked only after + * it passes, and an unknown scope is refused before either domain reader + * runs — the two whitelists must never blend into one combined read. */ import { loadSecrets } from "../../linear.ts"; import { loadMachineRepoTracking, grants, type RepoTracking } from "../../repo-tracking.ts"; import { loadOrCreateApiToken, tokenOk } from "../api-auth.ts"; +import { readSecret, createRealSecretsExecSeam, type SecretsSeams } from "../../secrets/store.ts"; +import { createRealAgeKeySeam } from "../../home/age-key.ts"; import type { Commands, ForgeSlug } from "../../../packages/rt-client/src/commands.ts"; import type { HandlerContext, HandlerMap, TypedHandlers } from "./types.ts"; +const DECK_SECRET_DOMAIN = "deck"; +const DECK_SECRET_KEYS = ["cfApiToken", "cfZoneId"] as const; + +let realDeckSecretsSeamsSingleton: SecretsSeams | null = null; + +/** Lazily-built real seams, separate from lib/linear.ts's `rt`-domain singleton since this reads a different domain file. */ +function defaultDeckSecretsSeams(): SecretsSeams { + return realDeckSecretsSeamsSingleton ??= { + ageKeySeam: createRealAgeKeySeam(), + execSeam: createRealSecretsExecSeam(), + }; +} + +async function loadDeckSecrets(): Promise<{ cfApiToken?: string; cfZoneId?: string }> { + const seams = defaultDeckSecretsSeams(); + const out: { cfApiToken?: string; cfZoneId?: string } = {}; + for (const key of DECK_SECRET_KEYS) { + const value = await readSecret(DECK_SECRET_DOMAIN, key, seams); + if (value !== null) out[key] = value; + } + return out; +} + const SECRETS_KEY: Record = { gitlab: "gitlabToken", github: "githubToken", @@ -45,8 +78,10 @@ const SECRETS_KEY: Record = { export interface SecretsHandlerOverrides { tracking?: () => RepoTracking; secrets?: () => { gitlabToken?: string; githubToken?: string } | Promise<{ gitlabToken?: string; githubToken?: string }>; - /** Defaults to `loadSecrets` (the full encrypted-store + plaintext-fallback loader) for secrets:read. */ + /** Defaults to `loadSecrets` (the full encrypted-store + plaintext-fallback loader) for secrets:read's "extension" scope. */ extensionSecrets?: () => Promise<{ linearApiKey?: string; gitlabToken?: string }>; + /** Defaults to `loadDeckSecrets` (the `deck` encrypted domain) for secrets:read's "deck" scope. */ + deckSecrets?: () => Promise<{ cfApiToken?: string; cfZoneId?: string }>; /** Defaults to `loadOrCreateApiToken` (the real ~/.mattstack/rt/api-token, shared with api-auth.ts). */ apiToken?: () => string; } @@ -62,6 +97,7 @@ export function createSecretsHandlers( const tracking = overrides.tracking ?? loadMachineRepoTracking; const secrets = overrides.secrets ?? loadSecrets; const extensionSecrets = overrides.extensionSecrets ?? loadSecrets; + const deckSecrets = overrides.deckSecrets ?? loadDeckSecrets; const apiToken = overrides.apiToken ?? (() => loadOrCreateApiToken()); return { @@ -101,11 +137,20 @@ export function createSecretsHandlers( return { ok: false as const, error: "bad-token" }; } - const all = await extensionSecrets(); + const scope = payload?.scope ?? "extension"; const data: Commands["secrets:read"]["data"] = {}; - if (all.linearApiKey) data.linearApiKey = all.linearApiKey; - if (all.gitlabToken) data.gitlabToken = all.gitlabToken; - ctx.log.debug({ keys: Object.keys(data) }, "secrets:read"); + if (scope === "extension") { + const all = await extensionSecrets(); + if (all.linearApiKey) data.linearApiKey = all.linearApiKey; + if (all.gitlabToken) data.gitlabToken = all.gitlabToken; + } else if (scope === "deck") { + const all = await deckSecrets(); + if (all.cfApiToken) data.cfApiToken = all.cfApiToken; + if (all.cfZoneId) data.cfZoneId = all.cfZoneId; + } else { + return { ok: false as const, error: "bad-scope" }; + } + ctx.log.debug({ scope, keys: Object.keys(data) }, "secrets:read"); return { ok: true as const, data }; }, }; diff --git a/packages/rt-client/package.json b/packages/rt-client/package.json index d33b6fc0..de17a716 100644 --- a/packages/rt-client/package.json +++ b/packages/rt-client/package.json @@ -40,7 +40,8 @@ }, "scripts": { "build": "bun build src/index.ts --outdir dist --target node --format esm --packages external && tsc -p tsconfig.json", - "check-types": "tsc --noEmit -p tsconfig.json" + "check-types": "tsc --noEmit -p tsconfig.json", + "prepack": "bun run build" }, "publishConfig": { "access": "public" diff --git a/packages/rt-client/src/commands.ts b/packages/rt-client/src/commands.ts index b463aae1..403f6b94 100644 --- a/packages/rt-client/src/commands.ts +++ b/packages/rt-client/src/commands.ts @@ -71,19 +71,27 @@ export interface Commands { */ "secrets:forge-token": { payload: { repoName: string; forge: ForgeSlug }; data: ForgeTokenData }; /** - * The whitelisted subset of `Secrets` the VS Code extension reads directly - * (RT-32): only linearApiKey and gitlabToken, both optional (present only - * when set). Not a general secrets export — extend the whitelist here, in - * lockstep with lib/daemon/handlers/secrets.ts and - * extensions/vscode/rt-context/src/secrets.ts, if a consumer needs another key. + * A per-`scope` whitelisted subset of secrets, each scope reading its own + * encrypted domain (RT-32, extended by Task 3 for deck): "extension" + * (default, so the VS Code extension needs no change) is linearApiKey/ + * gitlabToken from the `rt` domain; "deck" is cfApiToken/cfZoneId from the + * `deck` domain. Both optional per key (present only when set). Not a + * general secrets export — extend a whitelist here, in lockstep with + * lib/daemon/handlers/secrets.ts and (for "extension") + * extensions/vscode/rt-context/src/secrets.ts, if a consumer needs another + * key. * * `token` is required and checked in the HANDLER (not a transport-layer * gate alone), since this verb is reachable over the unauthenticated unix * socket too — see lib/daemon/handlers/secrets.ts's doc comment. HTTP * callers get it forwarded automatically from their X-RT-Token header; - * socket callers must read ~/.mattstack/rt/api-token themselves. + * socket callers must read ~/.mattstack/rt/api-token themselves. The gate + * applies identically to every scope. */ - "secrets:read": { payload: { token?: string }; data: { linearApiKey?: string; gitlabToken?: string } }; + "secrets:read": { + payload: { token?: string; scope?: "extension" | "deck" }; + data: { linearApiKey?: string; gitlabToken?: string; cfApiToken?: string; cfZoneId?: string }; + }; "events:emit": { payload: { topic: string; payload?: unknown }; data: { id: number } }; "events:wait": { payload: { pattern: string; after?: number; waitMs?: number }; data: { events: EventsBusEvent[]; cursor: number } }; "events:list": { payload: { pattern: string; after?: number; limit?: number }; data: { events: EventsBusEvent[]; cursor: number } }; diff --git a/packages/rt-client/test/dist-freshness.test.ts b/packages/rt-client/test/dist-freshness.test.ts new file mode 100644 index 00000000..ade65f40 --- /dev/null +++ b/packages/rt-client/test/dist-freshness.test.ts @@ -0,0 +1,28 @@ +/** + * Guards against a stale or broken dist/ silently breaking `file:` + * consumers (mr-board, gitq): those install by copying whatever dist/ + * currently holds, so `prepack` alone (npm pack/publish only) doesn't cover + * the local dev-linking path. Rebuilding here (cheap — a few seconds) before + * asserting turns a broken build script, not just a stale checked-in dist/, + * into a test failure. + */ + +import { describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "fs"; +import { join } from "path"; + +const pkgDir = join(import.meta.dir, ".."); +const distIndexDts = join(pkgDir, "dist", "index.d.ts"); + +describe("dist/ freshness", () => { + test("the build script regenerates dist/index.d.ts and it declares getSetting", () => { + const build = Bun.spawnSync(["bun", "run", "build"], { cwd: pkgDir, stdout: "pipe", stderr: "pipe" }); + if (build.exitCode !== 0) { + throw new Error(`bun run build failed:\n${build.stderr.toString()}`); + } + + expect(existsSync(distIndexDts)).toBe(true); + const contents = readFileSync(distIndexDts, "utf8"); + expect(contents).toContain("getSetting"); + }); +}); From ba806d6ee2089bae0751877978427bbb9b64672c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 09:24:44 -0500 Subject: [PATCH 2/4] =?UTF-8?q?RT:=20review=20fixes=20=E2=80=94=20detect-n?= =?UTF-8?q?ot-repair=20dist=20guard,=20scope=20union=20type,=20drop=20task?= =?UTF-8?q?=20refs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dist-freshness test now builds into a temp dir and diffs against the on-disk dist/ instead of rebuilding it in place, so a stale dist/ actually fails the test; also checks a fresh build's commands.d.ts carries cfApiToken (catalog drift). secrets:read's data type is now a union of the two exact per-scope shapes instead of a merged bag of all four keys, so a caller narrowing the wrong scope's fields is a compile error. Drops lane/task references from source comments (invariant prose kept), fixes the deck-seams-singleton comment's rationale (module privacy, not domain binding), and adds coverage for a case-variant scope, a non-string scope, and a deck-reader throw surfacing as a transport error instead of a partial ok. Co-Authored-By: Claude Fable 5 --- .../vscode/rt-context/src/secretsMapping.ts | 2 +- lib/daemon/__tests__/secrets-handler.test.ts | 39 ++++++++- lib/daemon/handlers/secrets.ts | 19 +++-- packages/rt-client/src/commands.ts | 16 ++-- .../rt-client/test/dist-freshness.test.ts | 79 +++++++++++++++---- 5 files changed, 119 insertions(+), 36 deletions(-) diff --git a/extensions/vscode/rt-context/src/secretsMapping.ts b/extensions/vscode/rt-context/src/secretsMapping.ts index 2d2b633a..dcf0e3d4 100644 --- a/extensions/vscode/rt-context/src/secretsMapping.ts +++ b/extensions/vscode/rt-context/src/secretsMapping.ts @@ -14,7 +14,7 @@ export interface DaemonSecretsResponse { error?: string; } -/** The daemon's secrets:read verb whitelists exactly these two fields (lib/daemon/handlers/secrets.ts) — keep in lockstep. */ +/** The daemon's secrets:read verb's extension scope whitelists exactly these two fields (lib/daemon/handlers/secrets.ts) — keep in lockstep. */ export type DaemonSecretKey = 'linearApiKey' | 'gitlabToken'; /** diff --git a/lib/daemon/__tests__/secrets-handler.test.ts b/lib/daemon/__tests__/secrets-handler.test.ts index 472c1537..583a03fd 100644 --- a/lib/daemon/__tests__/secrets-handler.test.ts +++ b/lib/daemon/__tests__/secrets-handler.test.ts @@ -196,10 +196,10 @@ describe("secrets:read", () => { }); }); -// scope: "deck" (Task 3) reads a wholly different encrypted domain -// (lib/secrets/store.ts readSecret("deck", ...)) — the whitelist and the -// domain are both per-scope, so an rt-domain value seeded for the extension -// scope must never leak into a deck-scope read, and vice versa. +// scope: "deck" reads a wholly different encrypted domain (lib/secrets/ +// store.ts readSecret("deck", ...)) — the whitelist and the domain are both +// per-scope, so an rt-domain value seeded for the extension scope must +// never leak into a deck-scope read, and vice versa. describe("secrets:read scope", () => { test("deck scope returns only cfApiToken/cfZoneId, never an rt-domain key seeded for extension scope", async () => { const h = readHandler({ @@ -257,4 +257,35 @@ describe("secrets:read scope", () => { expect(deckCalled).toBe(false); }); + + test("a case-variant scope (\"Deck\") is refused, not treated as \"deck\"", async () => { + let deckCalled = false; + const h = readHandler({ deckSecrets: async () => { deckCalled = true; return {}; } }); + + const res = await h({ token: "test-token", scope: "Deck" as any }); + + expect(res).toEqual({ ok: false, error: "bad-scope" }); + expect(deckCalled).toBe(false); + }); + + test("a non-string scope (array) is refused, not coerced into a match", async () => { + let extensionCalled = false; + let deckCalled = false; + const h = readHandler({ + extensionSecrets: async () => { extensionCalled = true; return {}; }, + deckSecrets: async () => { deckCalled = true; return {}; }, + }); + + const res = await h({ token: "test-token", scope: ["deck"] as any }); + + expect(res).toEqual({ ok: false, error: "bad-scope" }); + expect(extensionCalled).toBe(false); + expect(deckCalled).toBe(false); + }); + + test("a deck-reader throw surfaces as a rejected promise (transport error), never a partial ok", async () => { + const h = readHandler({ deckSecrets: async () => { throw new Error("sops -d exploded"); } }); + + await expect(h({ token: "test-token", scope: "deck" })).rejects.toThrow("sops -d exploded"); + }); }); diff --git a/lib/daemon/handlers/secrets.ts b/lib/daemon/handlers/secrets.ts index f4f29441..f488290b 100644 --- a/lib/daemon/handlers/secrets.ts +++ b/lib/daemon/handlers/secrets.ts @@ -30,7 +30,7 @@ * need to change; a socket caller must read ~/.mattstack/rt/api-token * itself and pass it the same way. * - * `payload.scope` (Task 3, deck lane) adds a second whitelist alongside the + * `payload.scope` adds a second whitelist alongside the * extension's, each reading its own encrypted domain — "extension" (default, * so existing callers need no change) reads the `rt` domain's linearApiKey/ * gitlabToken; "deck" reads the `deck` domain's cfApiToken/cfZoneId. The @@ -52,7 +52,7 @@ const DECK_SECRET_KEYS = ["cfApiToken", "cfZoneId"] as const; let realDeckSecretsSeamsSingleton: SecretsSeams | null = null; -/** Lazily-built real seams, separate from lib/linear.ts's `rt`-domain singleton since this reads a different domain file. */ +/** Lazily-built real seams, private to this module — a separate instance from lib/linear.ts's own singleton only because module scope doesn't let this file reuse that one, not because the seams themselves are domain-bound (`readSecret` takes `domain` as a parameter). */ function defaultDeckSecretsSeams(): SecretsSeams { return realDeckSecretsSeamsSingleton ??= { ageKeySeam: createRealAgeKeySeam(), @@ -138,20 +138,23 @@ export function createSecretsHandlers( } const scope = payload?.scope ?? "extension"; - const data: Commands["secrets:read"]["data"] = {}; if (scope === "extension") { const all = await extensionSecrets(); + const data: { linearApiKey?: string; gitlabToken?: string } = {}; if (all.linearApiKey) data.linearApiKey = all.linearApiKey; if (all.gitlabToken) data.gitlabToken = all.gitlabToken; - } else if (scope === "deck") { + ctx.log.debug({ scope, keys: Object.keys(data) }, "secrets:read"); + return { ok: true as const, data }; + } + if (scope === "deck") { const all = await deckSecrets(); + const data: { cfApiToken?: string; cfZoneId?: string } = {}; if (all.cfApiToken) data.cfApiToken = all.cfApiToken; if (all.cfZoneId) data.cfZoneId = all.cfZoneId; - } else { - return { ok: false as const, error: "bad-scope" }; + ctx.log.debug({ scope, keys: Object.keys(data) }, "secrets:read"); + return { ok: true as const, data }; } - ctx.log.debug({ scope, keys: Object.keys(data) }, "secrets:read"); - return { ok: true as const, data }; + return { ok: false as const, error: "bad-scope" }; }, }; } diff --git a/packages/rt-client/src/commands.ts b/packages/rt-client/src/commands.ts index 403f6b94..929da858 100644 --- a/packages/rt-client/src/commands.ts +++ b/packages/rt-client/src/commands.ts @@ -72,12 +72,14 @@ export interface Commands { "secrets:forge-token": { payload: { repoName: string; forge: ForgeSlug }; data: ForgeTokenData }; /** * A per-`scope` whitelisted subset of secrets, each scope reading its own - * encrypted domain (RT-32, extended by Task 3 for deck): "extension" - * (default, so the VS Code extension needs no change) is linearApiKey/ - * gitlabToken from the `rt` domain; "deck" is cfApiToken/cfZoneId from the - * `deck` domain. Both optional per key (present only when set). Not a - * general secrets export — extend a whitelist here, in lockstep with - * lib/daemon/handlers/secrets.ts and (for "extension") + * encrypted domain: "extension" (default, so the VS Code extension needs + * no change) is linearApiKey/gitlabToken from the `rt` domain; "deck" is + * cfApiToken/cfZoneId from the `deck` domain. `data` is a union of the two + * exact per-scope shapes, not a merged bag of all four keys — that makes + * a caller narrowing on the wrong scope's fields a compile error instead + * of a silent `undefined`. Both optional per key (present only when set). + * Not a general secrets export — extend a whitelist here, in lockstep + * with lib/daemon/handlers/secrets.ts and (for "extension") * extensions/vscode/rt-context/src/secrets.ts, if a consumer needs another * key. * @@ -90,7 +92,7 @@ export interface Commands { */ "secrets:read": { payload: { token?: string; scope?: "extension" | "deck" }; - data: { linearApiKey?: string; gitlabToken?: string; cfApiToken?: string; cfZoneId?: string }; + data: { linearApiKey?: string; gitlabToken?: string } | { cfApiToken?: string; cfZoneId?: string }; }; "events:emit": { payload: { topic: string; payload?: unknown }; data: { id: number } }; "events:wait": { payload: { pattern: string; after?: number; waitMs?: number }; data: { events: EventsBusEvent[]; cursor: number } }; diff --git a/packages/rt-client/test/dist-freshness.test.ts b/packages/rt-client/test/dist-freshness.test.ts index ade65f40..3814122c 100644 --- a/packages/rt-client/test/dist-freshness.test.ts +++ b/packages/rt-client/test/dist-freshness.test.ts @@ -1,28 +1,75 @@ /** - * Guards against a stale or broken dist/ silently breaking `file:` - * consumers (mr-board, gitq): those install by copying whatever dist/ - * currently holds, so `prepack` alone (npm pack/publish only) doesn't cover - * the local dev-linking path. Rebuilding here (cheap — a few seconds) before - * asserting turns a broken build script, not just a stale checked-in dist/, - * into a test failure. + * Guards against a stale dist/ silently breaking rt-client's node/type + * consumers. package.json's "exports" map routes bun (`"bun": "./src/index.ts"`) + * straight to source, so bun consumers never see a stale dist/ at all — this + * only protects the "types" and "import"/"default" conditions (node + * consumers, and any type-checker that resolves through those conditions + * instead of the bun one), and `file:` consumers (mr-board, gitq), which + * install by copying whatever dist/ currently holds on disk. + * + * This must DETECT staleness, not repair it: it builds into a throwaway + * temp dir and diffs that fresh output against the dist/ actually sitting + * on disk, failing on any mismatch. It never writes into the real dist/ — + * a passing run proves dist/ is current; a failing one means someone needs + * to run `bun run build` (or prepack needs to, at publish time) before the + * checked-in copy is trustworthy again. */ -import { describe, expect, test } from "bun:test"; -import { existsSync, readFileSync } from "fs"; +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "fs"; +import { tmpdir } from "os"; import { join } from "path"; const pkgDir = join(import.meta.dir, ".."); -const distIndexDts = join(pkgDir, "dist", "index.d.ts"); +const onDiskDist = join(pkgDir, "dist"); + +let tmpOutDir: string | undefined; + +afterEach(() => { + if (tmpOutDir) rmSync(tmpOutDir, { recursive: true, force: true }); + tmpOutDir = undefined; +}); + +/** Builds JS + .d.ts into a fresh temp dir, entirely separate from the real dist/ — this never touches the on-disk copy under test. */ +function buildIntoTempDir(): string { + const outDir = mkdtempSync(join(tmpdir(), "rt-client-dist-check-")); + tmpOutDir = outDir; + + const bundle = Bun.spawnSync( + ["bun", "build", "src/index.ts", "--outdir", outDir, "--target", "node", "--format", "esm", "--packages", "external"], + { cwd: pkgDir, stdout: "pipe", stderr: "pipe" }, + ); + if (bundle.exitCode !== 0) throw new Error(`bun build failed:\n${bundle.stderr.toString()}`); + + const types = Bun.spawnSync( + ["bunx", "tsc", "-p", "tsconfig.json", "--outDir", outDir], + { cwd: pkgDir, stdout: "pipe", stderr: "pipe" }, + ); + if (types.exitCode !== 0) throw new Error(`tsc -p tsconfig.json failed:\n${types.stdout.toString()}${types.stderr.toString()}`); + + return outDir; +} describe("dist/ freshness", () => { - test("the build script regenerates dist/index.d.ts and it declares getSetting", () => { - const build = Bun.spawnSync(["bun", "run", "build"], { cwd: pkgDir, stdout: "pipe", stderr: "pipe" }); - if (build.exitCode !== 0) { - throw new Error(`bun run build failed:\n${build.stderr.toString()}`); + test("the committed dist/ matches a from-scratch build byte for byte", () => { + const freshDir = buildIntoTempDir(); + + if (!existsSync(onDiskDist)) { + throw new Error("dist/ is missing on disk — run `bun run build` in packages/rt-client"); } - expect(existsSync(distIndexDts)).toBe(true); - const contents = readFileSync(distIndexDts, "utf8"); - expect(contents).toContain("getSetting"); + for (const file of ["index.d.ts", "commands.d.ts"]) { + const fresh = readFileSync(join(freshDir, file), "utf8"); + const onDisk = readFileSync(join(onDiskDist, file), "utf8"); + if (fresh !== onDisk) { + throw new Error(`dist/${file} is stale — it no longer matches a fresh build. Run \`bun run build\` in packages/rt-client.`); + } + } + }); + + test("a fresh build's commands.d.ts carries the deck-scope catalog (cfApiToken) — catches whitelist/build drift", () => { + const freshDir = buildIntoTempDir(); + const contents = readFileSync(join(freshDir, "commands.d.ts"), "utf8"); + expect(contents).toContain("cfApiToken"); }); }); From eca86e6ae275ee0f4359fac89933aefcbcfd1a11 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 09:54:50 -0500 Subject: [PATCH 3/4] RT: transport.ts DEFAULT_SOCK resolves at call time, not module load Matches settings/paths.ts's call-time HOME convention: rtCommand now derives its default sock path fresh on every call instead of reading a module-load `homedir()` snapshot, so a test repointing process.env.HOME after this module has already loaded no longer silently falls through to the real ~/.mattstack/rt/rt.sock. Confirmed no consumer (checked mr-board and gitq on disk) imports the exported DEFAULT_SOCK constant directly, so it stays as a display-only snapshot while rtCommand itself no longer reads it. Co-Authored-By: Claude Fable 5 --- packages/rt-client/src/transport.ts | 18 +++++++++++++++--- packages/rt-client/test/transport.test.ts | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/rt-client/src/transport.ts b/packages/rt-client/src/transport.ts index 6d42c332..71fbdc2a 100644 --- a/packages/rt-client/src/transport.ts +++ b/packages/rt-client/src/transport.ts @@ -23,15 +23,27 @@ export interface RtClientOptions { // Duplicates the ~/.mattstack/rt layout: rt-client has no dependency on rt's // lib/, so this literal cannot import rtDir(). repo-tools/lib/rt-paths.ts is -// the authority — change there first, mirror here. -export const DEFAULT_SOCK = join(homedir(), ".mattstack", "rt", "rt.sock"); +// the authority — change there first, mirror here (same convention as +// settings/paths.ts's call-time `home()`). +function defaultSock(): string { + return join(process.env.HOME ?? homedir(), ".mattstack", "rt", "rt.sock"); +} + +/** + * Display-only: a module-load snapshot for callers that just want to show + * the default path (no consumer imports it today — checked). `rtCommand` + * itself never reads this constant; it calls `defaultSock()` fresh on every + * invocation so a test can repoint `process.env.HOME` at any time before + * calling, not only before this module first loads. + */ +export const DEFAULT_SOCK = defaultSock(); export async function rtCommand( cmd: string, payload: Record, opts: { sockPath?: string; timeoutMs?: number } = {}, ): Promise> { - const sockPath = opts.sockPath ?? DEFAULT_SOCK; + const sockPath = opts.sockPath ?? defaultSock(); try { const res = await fetch(`http://localhost/${cmd}`, { unix: sockPath, diff --git a/packages/rt-client/test/transport.test.ts b/packages/rt-client/test/transport.test.ts index 27567faa..03a2954d 100644 --- a/packages/rt-client/test/transport.test.ts +++ b/packages/rt-client/test/transport.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test, afterEach } from "bun:test"; +import { mkdtempSync, rmSync, realpathSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { rtCommand } from "../src/transport.ts"; @@ -41,4 +42,18 @@ describe("rtCommand", () => { expect(res.ok).toBe(false); expect(res.error).toContain(`rt daemon unreachable at ${sock}`); }); + + test("no sockPath resolves the default at call time from process.env.HOME, not a module-load snapshot", async () => { + const origHome = process.env.HOME; + const fakeHome = realpathSync(mkdtempSync(join(tmpdir(), "rt-client-transport-home-"))); + process.env.HOME = fakeHome; + try { + const res = await rtCommand("project-mrs:read", { repoName: "x" }, {}); + expect(res.ok).toBe(false); + expect(res.error).toContain(join(fakeHome, ".mattstack", "rt", "rt.sock")); + } finally { + process.env.HOME = origHome; + rmSync(fakeHome, { recursive: true, force: true }); + } + }); }); From 8ff86c74b6c725a5bc386f6898aaa115430edc2f Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Fri, 21 Aug 2026 10:19:37 -0500 Subject: [PATCH 4/4] RT: dist-freshness test covers the full dist tree, not just two .d.ts files Re-docced: dist/ is gitignored (never "committed"), and prepack only rebuilds it for npm publish -- it does nothing for the file:-consumer dev-linking path this test actually protects. Widened the comparison to every emitted file (recursive listing + byte compare), so a stale dist/index.js -- the original failure class -- now fails the test instead of shipping silently; a missing dist/ (a fresh clone's starting state) gets its own message naming `bun run build` as the fix. Co-Authored-By: Claude Fable 5 --- .../rt-client/test/dist-freshness.test.ts | 79 +++++++++++++------ 1 file changed, 56 insertions(+), 23 deletions(-) diff --git a/packages/rt-client/test/dist-freshness.test.ts b/packages/rt-client/test/dist-freshness.test.ts index 3814122c..45515d88 100644 --- a/packages/rt-client/test/dist-freshness.test.ts +++ b/packages/rt-client/test/dist-freshness.test.ts @@ -1,24 +1,31 @@ /** - * Guards against a stale dist/ silently breaking rt-client's node/type - * consumers. package.json's "exports" map routes bun (`"bun": "./src/index.ts"`) - * straight to source, so bun consumers never see a stale dist/ at all — this - * only protects the "types" and "import"/"default" conditions (node - * consumers, and any type-checker that resolves through those conditions - * instead of the bun one), and `file:` consumers (mr-board, gitq), which - * install by copying whatever dist/ currently holds on disk. + * Guards against a stale on-disk dist/ silently breaking rt-client's + * node/type consumers. package.json's "exports" map routes bun + * (`"bun": "./src/index.ts"`) straight to source, so bun consumers never see + * dist/ at all — this only protects the "types" and "import"/"default" + * conditions (node consumers, and any type-checker that resolves through + * those conditions instead of the bun one). + * + * dist/ is a GITIGNORED build artifact, not something committed to git — + * `file:` consumers (mr-board, gitq) install by copying whatever dist/ + * happens to be sitting on disk at install time. `prepack` (package.json) + * only rebuilds dist/ for `npm pack`/publish; it does nothing for that + * local dev-linking path, so an on-disk dist/ that's drifted from src/ is + * invisible to every other guard in this repo. * * This must DETECT staleness, not repair it: it builds into a throwaway - * temp dir and diffs that fresh output against the dist/ actually sitting - * on disk, failing on any mismatch. It never writes into the real dist/ — - * a passing run proves dist/ is current; a failing one means someone needs - * to run `bun run build` (or prepack needs to, at publish time) before the - * checked-in copy is trustworthy again. + * temp dir and diffs that fresh output — file listing AND byte content of + * every emitted file (the JS bundle, every .d.ts) — against dist/ actually + * sitting on disk, failing on any mismatch. It never writes into the real + * dist/. A passing run proves the on-disk dist/ is current; a failing one + * (including a missing dist/ — the state a fresh clone starts in) names + * `bun run build` as the fix. */ import { afterEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, rmSync } from "fs"; +import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "fs"; import { tmpdir } from "os"; -import { join } from "path"; +import { join, relative } from "path"; const pkgDir = join(import.meta.dir, ".."); const onDiskDist = join(pkgDir, "dist"); @@ -50,19 +57,45 @@ function buildIntoTempDir(): string { return outDir; } -describe("dist/ freshness", () => { - test("the committed dist/ matches a from-scratch build byte for byte", () => { - const freshDir = buildIntoTempDir(); +/** Every file under `root`, as paths relative to `root`, sorted — recurses into subdirs (dist/settings/*.d.ts included). */ +function listFilesRecursive(root: string): string[] { + const out: string[] = []; + const walk = (dir: string) => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) walk(full); + else out.push(relative(root, full)); + } + }; + walk(root); + return out.sort(); +} +describe("dist/ freshness", () => { + test("the on-disk dist/ matches a fresh build — every emitted file, not just index.d.ts", () => { if (!existsSync(onDiskDist)) { - throw new Error("dist/ is missing on disk — run `bun run build` in packages/rt-client"); + throw new Error( + "dist/ does not exist on disk. It's gitignored (a build artifact, not checked into git), and " + + "file: consumers (mr-board, gitq) install by copying whatever's there — run `bun run build` " + + "in packages/rt-client, then rerun this test.", + ); } - for (const file of ["index.d.ts", "commands.d.ts"]) { - const fresh = readFileSync(join(freshDir, file), "utf8"); - const onDisk = readFileSync(join(onDiskDist, file), "utf8"); - if (fresh !== onDisk) { - throw new Error(`dist/${file} is stale — it no longer matches a fresh build. Run \`bun run build\` in packages/rt-client.`); + const freshDir = buildIntoTempDir(); + const freshFiles = listFilesRecursive(freshDir); + const onDiskFiles = listFilesRecursive(onDiskDist); + + expect(onDiskFiles).toEqual(freshFiles); + + for (const file of freshFiles) { + const fresh = readFileSync(join(freshDir, file)); + const onDisk = readFileSync(join(onDiskDist, file)); + if (!fresh.equals(onDisk)) { + throw new Error( + `dist/${file} is stale — it no longer matches a fresh build. dist/ is a gitignored artifact ` + + "that file: consumers copy as-is; prepack only regenerates it for npm publish, not for local " + + "dev-linking. Run `bun run build` in packages/rt-client.", + ); } } });