From a5f53a5cb2623dbdd7ced250cfee02105699adc8 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 13 Oct 2026 20:16:16 +0200 Subject: [PATCH 01/32] fix(mcp-manager): a global Claude disable denies the server in settings.json instead of deleting it, unless a project runs it through its own entry (cherry picked from commit 763f503ecb28761a1c156a10af494497a9f5402c) --- src/mcp-manager/README.md | 115 +++-- .../__tests__/claude-global-disable.test.ts | 179 +++++-- .../__tests__/claude-project-override.test.ts | 54 ++- src/mcp-manager/commands/rename.ts | 9 +- src/mcp-manager/utils/providers/claude.ts | 441 +++++++++++------- src/mcp-manager/utils/providers/grok.ts | 46 ++ 6 files changed, 611 insertions(+), 233 deletions(-) diff --git a/src/mcp-manager/README.md b/src/mcp-manager/README.md index 71222353a1..abab3eda1a 100644 --- a/src/mcp-manager/README.md +++ b/src/mcp-manager/README.md @@ -157,16 +157,30 @@ The tool uses a unified configuration file at `~/.genesis-tools/mcp-manager/conf - Supports global and project-specific server configurations - Supports stdio, SSE, and HTTP transports -- **Global disable = removal from `mcpServers`** (see below) +- **Global disable = a `deniedMcpServers` entry in `~/.claude/settings.json`** + (see below); the config stays in `mcpServers` #### Claude enable/disable semantics -Verified against the Claude Code binary: Claude Code reads `disabledMcpServers` -**only** from per-project entries (`.projects[].disabledMcpServers`). The -**top-level `disabledMcpServers` key is never read by Claude Code** — it is an -mcp-manager-only marker. The only mechanism Claude Code honors globally -(including future projects) is the server **not existing in `mcpServers` at -all**. +Verified on Claude Code 2.1.296 (research note +`2026-10-13-ClaudeCodeGlobalMcpSwitch.md`): the only +global switch for one server is a `deniedMcpServers: [{ "serverName": "" }]` +entry in `~/.claude/settings.json`. It covers every project, also one registered +later, and the config stays in `mcpServers`. Claude Code reads +`disabledMcpServers` **only** from the current project entry +(`.projects[].disabledMcpServers`); the **top-level `disabledMcpServers` key +in `~/.claude.json` is never read**. mcp-manager no longer writes it, and `sync` +removes it. + +What a denied server looks like in Claude Code: + +- It is ABSENT from `claude mcp list` and `/mcp`, not shown as "disabled"; + `claude mcp get ` says `No MCP server named ""`. +- `/mcp` cannot turn it back on: the error blames "enterprise managed policy", + although the entry is in your user settings. Use `tools mcp-manager enable`. +- The deny matches the name in every scope, so a project-scope entry or a + `.mcp.json` server of the same name is blocked too. A per-project enable + therefore cannot override a global disable; the command warns about this. **Global means the global switch only.** A global disable, a global enable, an install and a `sync` never write the per-project `disabledMcpServers` lists @@ -175,21 +189,22 @@ exception is `enable --clear-project-overrides` (below). A dry run of `sync` once wanted to add one disabled server to the lists of all 98 projects (about 640 diff lines). That is why the old sweep was removed on 2026-10-13. -mcp-manager therefore implements a TRUE global disable for Claude: - - **Global disable** (`tools mcp-manager disable --provider claude`): - 1. Preserves the server's full config in the unified config with - `_meta.enabled.claude = false` (imports it there first if missing — - the entry is never removed unless it is safely preserved). If the - config cannot be preserved, the entry stays and Claude Code still loads - the server everywhere; a warning says so. - 2. Keeps the top-level marker. - 3. **Removes the entry from `mcpServers`** in `~/.claude.json`. -- **Global enable** restores the entry into `mcpServers` from the unified - config and drops the top-level marker. It does not change project lists. - When some projects still list the server (old entries from the removed - sweep, or a per-project disable), the server stays off in those projects, - and one warning gives the count and the two commands below. + 1. Adds `{ "serverName": "" }` to `deniedMcpServers` in + `~/.claude/settings.json` (once; other keys and entries stay). This write + comes first: if you reject it, nothing else is written. + 2. Records `_meta.enabled.claude = false` in the unified config (imports the + config there first if it is missing). + 3. Keeps the entry in `mcpServers`. An entry that an older delete-based + disable removed is put back from the unified config, with one exception + for Grok (below). + 4. Drops the name from the top-level marker. +- **Global enable** puts the entry back into `mcpServers` when it is missing, + then removes exactly the `serverName` deny entry (a `serverUrl` pattern is + never touched; the key goes when the list is empty). It does not change + project lists. When some projects still list the server (old entries from + the removed sweep, or a per-project disable), the server stays off in those + projects, and one warning gives the count and the two commands below. - **Global enable with `--clear-project-overrides`** (`tools mcp-manager enable -p claude --clear-project-overrides`) also takes the name off every `.projects[].disabledMcpServers` list @@ -202,42 +217,52 @@ mcp-manager therefore implements a TRUE global disable for Claude: - **Per-project disable or enable** (`--project `) is the only command that writes a project's `disabledMcpServers` list, and only for the named projects. The global entry stays in `mcpServers`. -- **`list`** shows globally-disabled servers (absent from `~/.claude.json`) - as `disabled` for claude — sourced from the unified config — instead of - "not installed". -- **`sync`** treats `_meta.enabled.claude === false` as disabled-by-absence: - such servers are never (re)installed into `mcpServers`, and drifted - entries are removed. For `true`, `false` or an unset state, `sync` writes - no project list. For a per-project map (`{ "/abs/path": false }`), it writes - only the lists of the projects the map names, and creates no project entry. -- **`sync-from-providers`** does not interpret the absence of a - globally-disabled server as "user deleted it" — the unified config entry - and its `_meta.enabled.claude = false` flag are preserved. +- **`list` and `show`** read the global state from `deniedMcpServers`. A server + that is still absent from `~/.claude.json` with `_meta.enabled.claude = false` + shows as `disabled`, sourced from the unified config. +- **`sync`**: `_meta.enabled.claude === false` puts the config into + `mcpServers` and denies the name; `true` removes its `serverName` deny entry; + an unset state or a per-project map leaves the denylist as it is. The first + sync after this change therefore migrates every server the old + delete-based disable removed: config back, deny added, no change in what + Claude Code loads. The denylist is written first; if you reject that write, + `~/.claude.json` is not written either. For `true`, `false` or an unset + state, `sync` writes no project list. For a per-project map + (`{ "/abs/path": false }`), it writes only the lists of the projects the map + names, and creates no project entry. +- **Grok.** Grok loads `~/.claude.json` top-level `mcpServers` too, and + Claude's deny does not reach Grok. So an entry for a server that stays off in + Claude is put back only when Grok would not start it: Grok has its own + `config.toml` table for the name, lists it in `disabled_mcp_servers`, has + `[compat.claude] mcps = false`, or the unified config enables it for Grok. + Otherwise the entry stays out of `mcpServers` with a warning, and the deny + still keeps it off in Claude. A global ENABLE restores the entry + regardless: the server is meant to run, and Grok sees it as it always did. +- **`sync-from-providers`** keeps `_meta.enabled.claude = false` for a denied + server, and does not interpret a missing entry as "user deleted it". +- **`remove`** also removes the server's `serverName` deny entry, so a later + server of the same name is not blocked. **`rename`** moves the deny to the + new name. #### Per-project enable override (claude) -A global disable can be overridden for individual projects: - ```bash tools mcp-manager enable -p claude --project /abs/path [--project /other] [-y] ``` - Installs the server's claude-format config into - `.projects[].mcpServers.` in `~/.claude.json` — Claude Code - DOES honor project-scope entries there (it's the same storage - `claude mcp add -s local` uses) — and removes the name from that project's + `.projects[].mcpServers.` in `~/.claude.json` when the global + entry is missing, and removes the name from that project's `disabledMcpServers` list. -- The global state stays disabled: the server remains absent from the global - `mcpServers` and `_meta.enabled.claude` stays `false`. The override is - tracked solely by the presence of the project-scope entry — no extra - metadata. -- **Override-awareness:** a global disable and `sync` never delete the +- ⚠️ Against a global disable this no longer helps: the deny blocks the name in + every scope, project-scope entries included, and the command warns. To run a + server in some projects only, enable it globally and disable it with + `--project` where you do not want it. +- The override is tracked solely by the presence of the project-scope entry — + no extra metadata. A global disable and `sync` never delete the project-scope entry, and never put the name back on that project's `disabledMcpServers` list (a per-project map in `sync` skips that project too). `sync` refreshes the entry's config from the unified config. - `sync-from-providers` keeps `_meta.enabled.claude === false` even though - the override projects report the server as enabled. -- `list` shows such servers as `disabled globally, enabled in N project(s)`. - Undo with `tools mcp-manager disable -p claude --project ` — removes the project-scope entry and puts the name back on that project's disabled list. diff --git a/src/mcp-manager/commands/__tests__/claude-global-disable.test.ts b/src/mcp-manager/commands/__tests__/claude-global-disable.test.ts index 64572e1fd9..aa125aa3b7 100644 --- a/src/mcp-manager/commands/__tests__/claude-global-disable.test.ts +++ b/src/mcp-manager/commands/__tests__/claude-global-disable.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; -import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync, writeSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync, writeSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { atomicWriteIo } from "@genesiscz/utils/storage/atomic-file"; @@ -75,6 +75,22 @@ describe("claude provider TRUE global disable", () => { return SafeJSON.parse(readFileSync(claudeJsonPath(), "utf-8")) as ClaudeGenericConfig & Record; }; + const userSettingsPath = () => join(homeDir, ".claude", "settings.json"); + + const writeUserSettings = (settings: Record) => { + mkdirSync(join(homeDir, ".claude"), { recursive: true }); + writeFileSync(userSettingsPath(), `${SafeJSON.stringify(settings, null, 2)}\n`); + }; + + /** `deniedMcpServers` of the user settings: the real global switch of Claude Code. */ + const readDenied = (): unknown => { + if (!existsSync(userSettingsPath())) { + return undefined; + } + + return (SafeJSON.parse(readFileSync(userSettingsPath(), "utf-8")) as Record).deniedMcpServers; + }; + const writeUnified = async (config: UnifiedMCPConfig) => { const storage = new Storage("mcp-manager"); await storage.ensureDirs(); @@ -113,20 +129,28 @@ describe("claude provider TRUE global disable", () => { }); describe("disableServer(name, null) — global", () => { - // Intended change 2026-10-13: a global disable no longer sweeps the name into every - // project's disabledMcpServers list. Global means the global switch only. - it("removes the entry from mcpServers, keeps the top-level marker, leaves every project alone", async () => { + // Intended change 2026-10-13 (V-2): the global switch Claude Code honours is the + // user-settings deniedMcpServers list. The entry stays in mcpServers, and the top-level + // disabledMcpServers marker (which Claude Code never reads) is dropped. + it("denies the server in settings.json, keeps its entry, drops the marker, leaves every project alone", async () => { writeClaudeJson(claudeFixture()); await writeUnified(unifiedFixture()); + writeUserSettings({ theme: "dark" }); const provider = new ClaudeProvider(); await provider.disableServer("serena", null); const claude = readClaudeJson(); - expect(claude.mcpServers?.serena).toBeUndefined(); // TRUE global disable + expect(claude.mcpServers?.serena).toEqual(SERENA); // config kept + expect(readDenied()).toEqual([{ serverName: "serena" }]); // the global switch + expect((SafeJSON.parse(readFileSync(userSettingsPath(), "utf-8")) as Record).theme).toBe( + "dark" + ); expect(claude.mcpServers?.github).toBeDefined(); // untouched sibling - expect(claude.disabledMcpServers).toContain("serena"); // mcp-manager marker + expect(claude.disabledMcpServers).toBeUndefined(); // the empty marker list is gone expect(claude.projects).toEqual(claudeFixture().projects as ClaudeGenericConfig["projects"]); // no sweep + expect((await provider.listServers()).find((row) => row.name === "serena")?.enabled).toBe(false); + expect(await provider.isServerEnabled("serena")).toBe(false); // Unrelated state preserved (read-modify-write) expect(claude.numStartups).toBe(5); expect((claude.projects?.["/proj/a"] as Record | undefined)?.history).toEqual([ @@ -152,10 +176,11 @@ describe("claude provider TRUE global disable", () => { expect(unified.mcpServers.serena.args).toEqual(["serena"]); expect(unified.mcpServers.serena._meta?.enabled?.claude).toBe(false); - expect(readClaudeJson().mcpServers?.serena).toBeUndefined(); + expect(readClaudeJson().mcpServers?.serena).toBeDefined(); // kept: the deny is the switch + expect(readDenied()).toEqual([{ serverName: "serena" }]); }); - it("does NOT remove the entry when it cannot be preserved in the unified config", async () => { + it("still denies the server when the unified config write is rejected; nothing is removed", async () => { writeClaudeJson(claudeFixture()); await writeUnified({ mcpServers: {} }); @@ -166,11 +191,59 @@ describe("claude provider TRUE global disable", () => { await provider.disableServer("serena", null); const claude = readClaudeJson(); - expect(claude.mcpServers?.serena).toBeDefined(); // kept — no data loss - expect(claude.disabledMcpServers).toContain("serena"); // marker still applied + expect(claude.mcpServers?.serena).toBeDefined(); // the config never leaves ~/.claude.json + expect(readDenied()).toEqual([{ serverName: "serena" }]); expect(claude.projects).toEqual(claudeFixture().projects as ClaudeGenericConfig["projects"]); - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("NOT removed")); - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("still loads it in every project")); + }); + + it("puts back an entry an older delete-based disable removed, unless Grok would start it", async () => { + const claude = claudeFixture(); + delete (claude.mcpServers as Record).serena; + delete (claude.mcpServers as Record).github; + writeClaudeJson(claude); + await writeUnified(unifiedFixture()); + // Grok reads ~/.claude.json mcpServers: github has a Grok table (Grok runs that one), + // serena has none, so a serena entry in ~/.claude.json would start serena in Grok. + mkdirSync(join(homeDir, ".grok"), { recursive: true }); + writeFileSync( + join(homeDir, ".grok", "config.toml"), + TOML.stringify({ mcp_servers: { github: { command: "bunx", enabled: false } } }) + ); + + await new ClaudeProvider().disableServers(["serena", "github"], null); + + expect(readClaudeJson().mcpServers?.github).toEqual(GITHUB); // put back + expect(readClaudeJson().mcpServers?.serena).toBeUndefined(); // kept out because of Grok + expect(readDenied()).toEqual([{ serverName: "serena" }, { serverName: "github" }]); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("'serena' stays out of mcpServers")); + }); + + it("a sync puts the entry back despite Grok when the unified config enables it for Grok", async () => { + const claude = claudeFixture(); + delete (claude.mcpServers as Record).serena; + writeClaudeJson(claude); + const unified = unifiedFixture(); + unified.mcpServers.serena._meta = { enabled: { claude: false, grok: true } }; + await writeUnified(unified); + mkdirSync(join(homeDir, ".grok"), { recursive: true }); + writeFileSync(join(homeDir, ".grok", "config.toml"), TOML.stringify({ disabled_mcp_servers: ["other"] })); + + await new ClaudeProvider().syncServers({ serena: unified.mcpServers.serena }); + + expect(readClaudeJson().mcpServers?.serena).toEqual(SERENA); // Grok is meant to run it + expect(readDenied()).toEqual([{ serverName: "serena" }]); // Claude keeps it off + }); + + it("writes nothing to ~/.claude.json when the deny write is rejected", async () => { + const claude = claudeFixture(); + delete (claude.mcpServers as Record).serena; + writeClaudeJson(claude); + await writeUnified(unifiedFixture()); + const before = readFileSync(claudeJsonPath(), "utf-8"); + setGlobalOptions({}); // no --yes: every confirmation is answered "no" in a non-TTY + + expect(await new ClaudeProvider().disableServers(["serena"], null)).toBe(WriteResult.Rejected); + expect(readFileSync(claudeJsonPath(), "utf-8")).toBe(before); // no restore without its deny }); }); @@ -220,6 +293,8 @@ describe("claude provider TRUE global disable", () => { it("restores the entry, drops the top-level marker, leaves project lists and warns with the count", async () => { const projects = await listedEverywhere(); + writeUserSettings({ deniedMcpServers: [{ serverName: "serena" }, { serverUrl: "https://other.test/*" }] }); + const provider = new ClaudeProvider(); await provider.enableServer("serena", null); @@ -227,7 +302,8 @@ describe("claude provider TRUE global disable", () => { expect(result.mcpServers?.serena).toBeDefined(); // restored expect(result.mcpServers?.serena.command).toBe("uvx"); expect((result.mcpServers?.serena as Record | undefined)?._meta).toBeUndefined(); // no _meta leak - expect(result.disabledMcpServers).not.toContain("serena"); + expect(result.disabledMcpServers).toBeUndefined(); // the marker is gone + expect(readDenied()).toEqual([{ serverUrl: "https://other.test/*" }]); // only its own entry lifted expect(result.projects).toEqual(projects as ClaudeGenericConfig["projects"]); // lists untouched expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("3 project(s)")); expect(logger.warn).toHaveBeenCalledWith( @@ -287,7 +363,31 @@ describe("claude provider TRUE global disable", () => { }); }); + describe("remove command", () => { + it("removeServers lifts the deny entry too, so a later server of that name is not blocked", async () => { + writeClaudeJson(claudeFixture()); + writeUserSettings({ deniedMcpServers: [{ serverName: "serena" }, { serverName: "other" }] }); + + await new ClaudeProvider().removeServers(["serena"]); + + expect(readClaudeJson().mcpServers?.serena).toBeUndefined(); + expect(readDenied()).toEqual([{ serverName: "other" }]); + }); + }); + describe("listServers", () => { + it("reads the global state from deniedMcpServers, not from the top-level marker", async () => { + const claude = claudeFixture(); + claude.disabledMcpServers = ["github"]; // ignored by Claude Code, and now by mcp-manager + writeClaudeJson(claude); + writeUserSettings({ deniedMcpServers: [{ serverName: "serena" }] }); + + const rows = await new ClaudeProvider().listServers(); + + expect(rows.find((row) => row.name === "serena")?.enabled).toBe(false); + expect(rows.find((row) => row.name === "github")?.enabled).toBe(true); + }); + it("reports a globally-disabled (absent) server as disabled, not missing", async () => { const claude = claudeFixture(); delete (claude.mcpServers as Record).serena; @@ -309,8 +409,12 @@ describe("claude provider TRUE global disable", () => { }); describe("syncServers (provider)", () => { - it("removes globally-disabled servers from mcpServers and keeps enabled ones", async () => { - writeClaudeJson(claudeFixture()); // serena still installed (drift) + // Intended change 2026-10-13 (V-2): false = in mcpServers AND denied; true lifts the deny. + it("denies a disabled server and keeps its entry, lifts the deny of an enabled one, drops the marker", async () => { + const claude = claudeFixture(); + claude.disabledMcpServers = ["serena", "github", "old-name"]; + writeClaudeJson(claude); + writeUserSettings({ deniedMcpServers: [{ serverName: "github" }] }); // stale deny of an enabled server const unified = unifiedFixture(); unified.mcpServers.serena._meta = { enabled: { claude: false } }; await writeUnified(unified); @@ -318,11 +422,24 @@ describe("claude provider TRUE global disable", () => { const provider = new ClaudeProvider(); await provider.syncServers(unified.mcpServers); - const claude = readClaudeJson(); - expect(claude.mcpServers?.serena).toBeUndefined(); // drift corrected - expect(claude.mcpServers?.github).toBeDefined(); - expect(claude.disabledMcpServers).toContain("serena"); - expect(claude.disabledMcpServers).not.toContain("github"); + const result = readClaudeJson(); + expect(result.mcpServers?.serena).toEqual(SERENA); + expect(result.mcpServers?.github).toBeDefined(); + expect(readDenied()).toEqual([{ serverName: "serena" }]); + expect(result.disabledMcpServers).toBeUndefined(); // Claude Code never read it + }); + + it("leaves the deny of a server with no claude decision as it is", async () => { + writeClaudeJson(claudeFixture()); + writeUserSettings({ deniedMcpServers: [{ serverName: "unset" }] }); + const unified = unifiedFixture(); + unified.mcpServers.unset = { type: "stdio", command: "unset-mcp" }; + await writeUnified(unified); + + await new ClaudeProvider().syncServers({ unset: unified.mcpServers.unset }); + + expect(readDenied()).toEqual([{ serverName: "unset" }]); + expect(readClaudeJson().mcpServers?.unset).toBeDefined(); }); }); @@ -348,7 +465,9 @@ describe("claude provider TRUE global disable", () => { const claude = readClaudeJson(); expect(claude.mcpServers?.github).toBeDefined(); // the sync really ran expect(claude.mcpServers?.unset).toBeDefined(); + // /proj/o runs serena through a project-scope entry, so serena keeps the older switch. expect(claude.mcpServers?.serena).toBeUndefined(); + expect(readDenied()).toBeUndefined(); // The only project change: the override entry is refreshed from the unified config. const expected = { ...projectsFixture(), @@ -380,7 +499,7 @@ describe("claude provider TRUE global disable", () => { }); describe("sync command", () => { - it("does not (re)install globally-disabled servers", async () => { + it("puts a disabled server back into mcpServers and denies it", async () => { writeClaudeJson({ numStartups: 5, mcpServers: {}, @@ -396,7 +515,9 @@ describe("claude provider TRUE global disable", () => { const claude = readClaudeJson(); expect(claude.mcpServers?.github).toBeDefined(); // enabled → installed - expect(claude.mcpServers?.serena).toBeUndefined(); // disabled → stays absent + expect(claude.mcpServers?.serena).toEqual(SERENA); // disabled → kept, denied + expect(readDenied()).toEqual([{ serverName: "serena" }]); + expect(claude.disabledMcpServers).toBeUndefined(); }); }); @@ -421,7 +542,7 @@ describe("claude provider TRUE global disable", () => { }); describe("rename command", () => { - it("renames a globally-disabled server in the unified config without installing it in claude", async () => { + it("renames a globally-disabled server: the new name is kept and denied, the old deny goes", async () => { writeClaudeJson({ mcpServers: {}, disabledMcpServers: ["serena"], @@ -431,6 +552,7 @@ describe("claude provider TRUE global disable", () => { await writeUnified({ mcpServers: { serena: { ...SERENA, _meta: { enabled: { claude: false } } } }, }); + writeUserSettings({ deniedMcpServers: [{ serverName: "serena" }] }); const provider = new ClaudeProvider(); await renameServer("serena", "serena-x", [provider]); @@ -441,20 +563,22 @@ describe("claude provider TRUE global disable", () => { expect(unified.mcpServers.serena).toBeUndefined(); const claude = readClaudeJson(); - expect(claude.mcpServers?.["serena-x"]).toBeUndefined(); // still globally disabled = absent + expect(claude.mcpServers?.["serena-x"]).toEqual(SERENA); // kept under its new name expect(claude.mcpServers?.serena).toBeUndefined(); + expect(readDenied()).toEqual([{ serverName: "serena-x" }]); // still off; no stale deny }); }); describe("disable/enable commands end-to-end (toggle flow)", () => { - it("disable --provider claude removes the entry and flips _meta to false", async () => { + it("disable --provider claude denies the server, keeps the entry and flips _meta to false", async () => { writeClaudeJson(claudeFixture()); await writeUnified(unifiedFixture()); const provider = new ClaudeProvider(); await disableCommand("serena", [provider], { provider: "claude" }); - expect(readClaudeJson().mcpServers?.serena).toBeUndefined(); + expect(readClaudeJson().mcpServers?.serena).toEqual(SERENA); + expect(readDenied()).toEqual([{ serverName: "serena" }]); const unified = await readUnified(); expect(unified.mcpServers.serena._meta?.enabled?.claude).toBe(false); expect(unified.mcpServers.serena.command).toBe("uvx"); @@ -475,7 +599,8 @@ describe("claude provider TRUE global disable", () => { const result = readClaudeJson(); expect(result.mcpServers?.serena).toBeDefined(); - expect(result.disabledMcpServers).not.toContain("serena"); + expect(result.disabledMcpServers).toBeUndefined(); + expect(readDenied()).toBeUndefined(); // nothing was denied, nothing is written const unifiedAfter = await readUnified(); expect(unifiedAfter.mcpServers.serena._meta?.enabled?.claude).toBe(true); diff --git a/src/mcp-manager/commands/__tests__/claude-project-override.test.ts b/src/mcp-manager/commands/__tests__/claude-project-override.test.ts index c8b86c871d..5febda3668 100644 --- a/src/mcp-manager/commands/__tests__/claude-project-override.test.ts +++ b/src/mcp-manager/commands/__tests__/claude-project-override.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setupStorageSandbox } from "@genesiscz/utils/storage/test-sandbox"; @@ -50,6 +50,13 @@ describe("claude per-project enable override for globally-disabled servers", () return SafeJSON.parse(readFileSync(claudeJsonPath(), "utf-8")) as ClaudeGenericConfig & Record; }; + const readDenied = (): unknown => { + const settingsPath = join(homeDir, ".claude", "settings.json"); + return existsSync(settingsPath) + ? (SafeJSON.parse(readFileSync(settingsPath, "utf-8")) as Record).deniedMcpServers + : undefined; + }; + const writeUnified = async (config: UnifiedMCPConfig) => { const storage = new Storage("mcp-manager"); await storage.ensureDirs(); @@ -147,6 +154,27 @@ describe("claude per-project enable override for globally-disabled servers", () }); // Intended change 2026-10-13: a global disable no longer sweeps project lists at all. + it("a later global disable keeps the older switch for an overridden server: no deny, entry absent", async () => { + writeClaudeJson({ + mcpServers: { serena: { ...SERENA }, github: { ...GITHUB } }, + projects: { + "/proj/a": { mcpServers: { serena: { ...SERENA } }, disabledMcpServers: [] }, + "/proj/b": { mcpServers: {}, disabledMcpServers: [] }, + }, + }); + await writeUnified(unifiedFixture()); + + await new ClaudeProvider().disableServers(["serena", "github"], null); + + const claude = readClaudeJson(); + expect(claude.mcpServers?.serena).toBeUndefined(); // overridden: absence is the switch + expect(claude.projects?.["/proj/a"].mcpServers?.serena).toBeDefined(); // the override keeps working + expect(claude.mcpServers?.github).toBeDefined(); // not overridden: kept and denied + expect(readDenied()).toEqual([{ serverName: "github" }]); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("'serena' keeps the older global switch")); + expect((await readUnified()).mcpServers.serena.command).toBe("uvx"); // config preserved + }); + it("a later global disable leaves every project entry alone, the override included", async () => { writeClaudeJson(globallyDisabledFixture()); await writeUnified(unifiedFixture()); @@ -177,8 +205,10 @@ describe("claude per-project enable override for globally-disabled servers", () expect(claude.projects?.["/proj/a"].mcpServers?.serena.command).toBe("uvx"); // Name NOT re-added to the override project's disabled list expect(claude.projects?.["/proj/a"].disabledMcpServers).not.toContain("serena"); - // Global disable still enforced (absent from mcpServers); other project lists untouched + // Global disable still enforced by absence: a deny (V-2) would match the project-scope + // entry too and switch /proj/a off, so an overridden server keeps the older switch. expect(claude.mcpServers?.serena).toBeUndefined(); + expect(readDenied()).toBeUndefined(); expect(claude.projects?.["/proj/b"].disabledMcpServers).toEqual(["serena"]); // Enabled sibling unaffected expect(claude.mcpServers?.github).toBeDefined(); @@ -218,6 +248,23 @@ describe("claude per-project enable override for globally-disabled servers", () expect(unified.mcpServers.serena._meta?.enabled?.claude).toBe(false); // global state untouched }); + it("a per-project enable of a DENIED server warns that the deny wins in every project", async () => { + writeClaudeJson({ + mcpServers: { serena: { ...SERENA }, github: { ...GITHUB } }, + projects: { "/proj/a": { mcpServers: {}, disabledMcpServers: [] } }, + }); + await writeUnified(unifiedFixture()); + mkdirSync(join(homeDir, ".claude"), { recursive: true }); + writeFileSync( + join(homeDir, ".claude", "settings.json"), + SafeJSON.stringify({ deniedMcpServers: [{ serverName: "serena" }] }) + ); + + await enableCommand("serena", [new ClaudeProvider()], { provider: "claude", project: ["/proj/a"] }); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("a per-project enable cannot turn it on")); + }); + it("listServers reports the global disable AND the per-project override", async () => { writeClaudeJson(globallyDisabledFixture()); await writeUnified(unifiedFixture()); @@ -254,7 +301,8 @@ describe("claude per-project enable override for globally-disabled servers", () await disableCommand("github", [provider, cursorProvider], { provider: "all" }); const claude = readClaudeJson(); - expect(claude.mcpServers?.github).toBeUndefined(); // globally disabled = removed + expect(claude.mcpServers?.github).toBeDefined(); // globally disabled = denied, config kept + expect(readDenied()).toEqual([{ serverName: "github" }]); const cursor = SafeJSON.parse(readFileSync(join(homeDir, ".cursor", "mcp.json"), "utf-8")) as { mcpServers?: Record; diff --git a/src/mcp-manager/commands/rename.ts b/src/mcp-manager/commands/rename.ts index 2e452cf3a3..2a5c66f361 100644 --- a/src/mcp-manager/commands/rename.ts +++ b/src/mcp-manager/commands/rename.ts @@ -284,8 +284,10 @@ async function renameServerInProvider( } // IMPORTANT: syncServers only adds/updates servers, it doesn't remove servers not in the map - // We need to explicitly remove the old server name from the provider config after syncing - if (hasOldServer && oldName !== newName) { + // We need to explicitly remove the old server name from the provider config after syncing. + // Claude can hold the old name only as a deny entry in ~/.claude/settings.json (an older + // delete-based disable left no config), so Claude always gets the removal. + if (oldName !== newName && (hasOldServer || provider.getName() === "claude")) { await removeServerFromProvider(provider, oldName); } @@ -327,6 +329,9 @@ async function removeServerFromProvider(provider: MCPProvider, serverName: strin } } await provider.writeConfig(claudeConfig); + // The old name's deny entry in ~/.claude/settings.json would block a later server of + // that name; removeServers lifts it (the entries above are already gone). + await provider.removeServers([serverName]); break; } case "codex": { diff --git a/src/mcp-manager/utils/providers/claude.ts b/src/mcp-manager/utils/providers/claude.ts index 0ca59608c8..e3ef3754d3 100644 --- a/src/mcp-manager/utils/providers/claude.ts +++ b/src/mcp-manager/utils/providers/claude.ts @@ -12,6 +12,7 @@ import { atomicWriteFileSync } from "@genesiscz/utils/storage/atomic-file"; import { matchGlob } from "@genesiscz/utils/string"; import chalk from "chalk"; import type { ClaudeGenericConfig, ClaudeMCPServerConfig } from "./claude.types.js"; +import { namesGrokWouldStartFromClaudeJson } from "./grok.js"; import { claudeEnabledPluginServers, readClaudeSettings } from "./plugins.js"; import type { EnableServersOptions, MCPServerInfo, UnifiedMCPServerConfig } from "./types.js"; import { MCPProvider, WriteResult } from "./types.js"; @@ -46,18 +47,55 @@ function isDeniedByClaude(denied: unknown[], registeredName: string, url: string }); } +/** Applied when either write applied; otherwise the two writes changed nothing. */ +function combineWrites(first: WriteResult, second: WriteResult): WriteResult { + if (second === WriteResult.Rejected) { + return second; + } + + return first === WriteResult.Applied || second === WriteResult.Applied + ? WriteResult.Applied + : WriteResult.NoChanges; +} + +/** How many projects run `name` through their own project-scope entry (an `enable --project` override). */ +function projectsOverriding(config: ClaudeGenericConfig, name: string): number { + return Object.values(config.projects ?? {}).filter((project) => project.mcpServers?.[name]).length; +} + +/** Drop names from the top-level `disabledMcpServers` marker, and the key once it is empty. */ +function dropTopLevelMarker(config: ClaudeGenericConfig, names: string[]): void { + if (!config.disabledMcpServers) { + return; + } + + config.disabledMcpServers = config.disabledMcpServers.filter((name) => !names.includes(name)); + + if (config.disabledMcpServers.length === 0) { + delete config.disabledMcpServers; + } +} + /** * Claude Desktop MCP provider. * Manages MCP servers in ~/.claude.json * - * IMPORTANT (verified against the Claude Code binary): Claude Code reads - * `disabledMcpServers` ONLY from per-project entries - * (`.projects[].disabledMcpServers`). The TOP-LEVEL `disabledMcpServers` - * key is NEVER read by Claude Code — it is an mcp-manager-only marker. The only - * mechanism Claude Code honors globally (including future projects) is the - * server NOT existing in `mcpServers` — so a TRUE global disable removes the - * entry (after preserving its full config in the unified config with - * `_meta.enabled.claude = false`), and a global enable restores it. + * The global switch (verified on Claude Code 2.1.296, research note + * 2026-10-13-ClaudeCodeGlobalMcpSwitch.md): a + * `deniedMcpServers: [{ "serverName": "" }]` entry in + * `~/.claude/settings.json`. It covers every project, also one registered later, + * and the config stays in `mcpServers`. A denied server is ABSENT from + * `claude mcp list` and `/mcp` (not shown as disabled), and `/mcp` cannot turn + * it back on ("blocked by enterprise managed policy"). The deny matches the + * name in every scope, so a project-scope entry of the same name is blocked too. + * The top-level `disabledMcpServers` key in `~/.claude.json` does nothing; + * mcp-manager no longer writes it and removes it on sync. Claude Code reads + * `disabledMcpServers` only from the current project entry. + * + * Grok reads `~/.claude.json` top-level `mcpServers` too, and Claude's deny does + * not reach Grok. So a server that stays off in Claude is put back into + * `mcpServers` only when Grok would not start it from there (see + * `namesGrokWouldStartFromClaudeJson`) or the unified config enables it for Grok. * * Global means the global switch only. A global disable, a global enable, an * install and a sync of a boolean or unset `_meta.enabled.claude` never write a @@ -83,10 +121,9 @@ export class ClaudeProvider extends MCPProvider { } shouldBeInstalled(serverConfig: UnifiedMCPServerConfig): boolean { - // A globally-disabled server (_meta.enabled.claude === false) must be - // ABSENT from ~/.claude.json — absence is the only global disable - // Claude Code honors. Everything else (enabled, per-project, unset) - // stays installed. + // A server with `_meta.enabled.claude === false` is put back into + // mcpServers by syncServers itself, which first checks that Grok would + // not start it from ~/.claude.json. Everything else installs up front. return serverConfig._meta?.enabled?.claude !== false; } @@ -126,18 +163,18 @@ export class ClaudeProvider extends MCPProvider { async listServers(): Promise { const config = await this.readConfig(); + const denied = await this.readDeniedEntries(); const servers: MCPServerInfo[] = []; const globalNames = new Set(); - // Global servers + // Global servers: off when the user-settings denylist names them. if (config.mcpServers) { - const disabledServers = new Set(config.disabledMcpServers || []); for (const [name, serverConfig] of Object.entries(config.mcpServers)) { globalNames.add(name); servers.push({ name, config: this.claudeToUnified(serverConfig), - enabled: !disabledServers.has(name), + enabled: !isDeniedByClaude(denied, name, serverConfig.url), provider: this.providerName, }); } @@ -156,7 +193,8 @@ export class ClaudeProvider extends MCPProvider { servers.push({ name, config: this.claudeToUnified(serverConfig), - enabled: !projectDisabled.has(name), + enabled: + !projectDisabled.has(name) && !isDeniedByClaude(denied, name, serverConfig.url), provider: `${this.providerName}:${projectPath}`, }); } @@ -165,9 +203,9 @@ export class ClaudeProvider extends MCPProvider { } } - // Globally-disabled servers are absent from ~/.claude.json entirely - // (absence is the only global disable Claude Code honors). Surface them - // from the unified config as disabled so they don't look "not installed". + // A server an older delete-based disable removed from ~/.claude.json (or one + // kept out because Grok would start it): surface it from the unified config + // as disabled so it does not look "not installed". const unified = await readUnifiedConfig(); for (const [name, entry] of Object.entries(unified.mcpServers)) { if (entry._meta?.enabled?.claude !== false) { @@ -225,56 +263,85 @@ export class ClaudeProvider extends MCPProvider { } } + /** The `deniedMcpServers` entries of the user settings; an unreadable file counts as none, with a warning. */ + private async readDeniedEntries(): Promise { + try { + return deniedEntries(await this.readSettings()); + } catch (error) { + logger.warn({ error, path: this.getSettingsPath() }, "mcp-manager: could not read Claude user settings"); + return []; + } + } + /** - * Claude Code's global switch for one plugin server is the user-settings denylist: - * `deniedMcpServers: [{ "serverName": "plugin::" }]`. The plugin stays enabled - * (verified on Claude Code 2.1.296: `claude mcp get plugin:expo:expo` no longer finds it). - * Enabling removes exactly those `serverName` entries, and the key when it ends up empty. + * Edit the user-settings denylist in memory: add `{ serverName }` for each name in `deny` that no + * entry names yet, and remove exactly the `serverName` entries for the names in `allow` (a + * `serverUrl` pattern is never touched). The key goes when the list ends up empty. Returns + * whether anything changed. */ - async setPluginServersEnabled(serverNames: string[], enabled: boolean): Promise { - const settings = await this.readSettings(); - const registered = new Set( - (await this.discoverPluginServers(settings)) - .filter((server) => serverNames.includes(server.info.name)) - .map((server) => server.registeredName) - ); + private applyDenyChanges(settings: Record, change: { deny: string[]; allow: string[] }): boolean { + const current = deniedEntries(settings); + const named = (entry: unknown): string | undefined => + isObject(entry) && typeof entry.serverName === "string" ? entry.serverName : undefined; + const kept = current.filter((entry) => { + const name = named(entry); + return name === undefined || !change.allow.includes(name); + }); + const present = new Set(kept.map(named)); + const added = [...new Set(change.deny)].filter((name) => !present.has(name)); - if (registered.size === 0) { - return WriteResult.NoChanges; + if (kept.length === current.length && added.length === 0) { + return false; } - const current = deniedEntries(settings); - const namedIn = (entry: unknown): boolean => - isObject(entry) && typeof entry.serverName === "string" && registered.has(entry.serverName); + const next = [...kept, ...added.map((serverName) => ({ serverName }))]; - if (enabled) { - const kept = current.filter((entry) => !namedIn(entry)); + if (next.length === 0) { + delete settings.deniedMcpServers; + } else { + settings.deniedMcpServers = next; + } - if (kept.length === current.length) { - return WriteResult.NoChanges; - } + return true; + } - if (kept.length === 0) { - delete settings.deniedMcpServers; - } else { - settings.deniedMcpServers = kept; - } - } else { - const present = new Set( - current - .filter(isObject) - .flatMap((entry) => (typeof entry.serverName === "string" ? [entry.serverName] : [])) - ); - const missing = [...registered].filter((name) => !present.has(name)); + /** Deny (`denied`) or allow the given registered server names in `~/.claude/settings.json`. */ + private async setDeniedServerNames(names: string[], denied: boolean): Promise { + const settings = await this.readSettings(); + const changed = this.applyDenyChanges( + settings, + denied ? { deny: names, allow: [] } : { deny: [], allow: names } + ); - if (missing.length === 0) { - return WriteResult.NoChanges; - } + return changed ? this.writeSettings(settings) : WriteResult.NoChanges; + } - settings.deniedMcpServers = [...current, ...missing.map((serverName) => ({ serverName }))]; + /** + * Claude Code's global switch for one plugin server is the user-settings denylist: + * `deniedMcpServers: [{ "serverName": "plugin::" }]`. The plugin stays enabled + * (verified on Claude Code 2.1.296: `claude mcp get plugin:expo:expo` no longer finds it). + * Enabling removes exactly those `serverName` entries, and the key when it ends up empty. + */ + async setPluginServersEnabled(serverNames: string[], enabled: boolean): Promise { + const settings = await this.readSettings(); + const registered = [ + ...new Set( + (await this.discoverPluginServers(settings)) + .filter((server) => serverNames.includes(server.info.name)) + .map((server) => server.registeredName) + ), + ]; + + if (registered.length === 0) { + return WriteResult.NoChanges; } - return this.writeSettings(settings); + const changed = this.applyDenyChanges( + settings, + enabled ? { deny: [], allow: registered } : { deny: registered, allow: [] } + ); + + return changed ? this.writeSettings(settings) : WriteResult.NoChanges; } /** Same flow as every harness write (diff, confirm or --yes, backup), aimed at settings.json. */ @@ -296,6 +363,7 @@ export class ClaudeProvider extends MCPProvider { } await this.backupManager.createBackup(settingsPath, this.providerName); + // Atomic like every harness write: a crash or a full disk leaves the old settings.json, never a short one. await mkdir(path.dirname(settingsPath), { recursive: true }); atomicWriteFileSync(settingsPath, newContent, { mode: 0o600 }); logger.info(chalk.green(`✓ Configuration written to ${settingsPath}`)); @@ -376,29 +444,11 @@ export class ClaudeProvider extends MCPProvider { } async enableServer(serverName: string, projectPath?: string | null): Promise { - const config = await this.readConfig(); - - if (projectPath === null || projectPath === undefined) { - // Global enable (no project specified = global) - await this.applyGlobalEnable([serverName], config); - } else { - await this.applyProjectEnable([serverName], projectPath, config); - } - - await this.writeConfig(config); + await this.enableServers([serverName], projectPath); } async disableServer(serverName: string, projectPath?: string | null): Promise { - const config = await this.readConfig(); - - if (projectPath === null || projectPath === undefined) { - // Global disable (no project specified = global) - await this.applyGlobalDisable([serverName], config); - } else { - this.applyProjectDisable([serverName], projectPath, config); - } - - await this.writeConfig(config); + await this.disableServers([serverName], projectPath); } async enableServerForAllProjects(serverName: string): Promise { @@ -409,6 +459,10 @@ export class ClaudeProvider extends MCPProvider { await this.disableServer(serverName, null); } + /** + * A global enable writes ~/.claude.json first (restore the entry), then lifts the deny in + * settings.json. A rejected second write leaves the server denied, so it stays off. + */ async enableServers( serverNames: string[], projectPath?: string | null, @@ -416,25 +470,45 @@ export class ClaudeProvider extends MCPProvider { ): Promise { const config = await this.readConfig(); - if (projectPath === null || projectPath === undefined) { - await this.applyGlobalEnable(serverNames, config, options); - } else { + if (projectPath !== null && projectPath !== undefined) { await this.applyProjectEnable(serverNames, projectPath, config); + return this.writeConfig(config); + } + + await this.applyGlobalEnable(serverNames, config, options); + const written = await this.writeConfig(config); + + if (written === WriteResult.Rejected) { + return written; } - return this.writeConfig(config); + return combineWrites(written, await this.setDeniedServerNames(serverNames, false)); } + /** + * A global disable denies the server in settings.json FIRST, then fixes ~/.claude.json (entry + * kept or put back, top-level marker dropped). A rejected deny writes nothing, so a disable + * can never restore an entry without its deny and turn the server on. A server that some + * project runs through its own project-scope entry is not denied (see `applyGlobalDisable`). + */ async disableServers(serverNames: string[], projectPath?: string | null): Promise { const config = await this.readConfig(); - if (projectPath === null || projectPath === undefined) { - await this.applyGlobalDisable(serverNames, config); - } else { + if (projectPath !== null && projectPath !== undefined) { this.applyProjectDisable(serverNames, projectPath, config); + return this.writeConfig(config); + } + + const overridden = serverNames.filter((name) => projectsOverriding(config, name) > 0); + const deniable = serverNames.filter((name) => !overridden.includes(name)); + const denied = deniable.length > 0 ? await this.setDeniedServerNames(deniable, true) : WriteResult.NoChanges; + + if (denied === WriteResult.Rejected) { + return denied; } - return this.writeConfig(config); + await this.applyGlobalDisable(serverNames, config, overridden); + return combineWrites(denied, await this.writeConfig(config)); } /** @@ -454,6 +528,15 @@ export class ClaudeProvider extends MCPProvider { config: ClaudeGenericConfig ): Promise { const existingProject = config.projects?.[projectPath]; + const denied = await this.readDeniedEntries(); + + for (const name of serverNames.filter((server) => isDeniedByClaude(denied, server, undefined))) { + logger.warn( + chalk.yellow( + `⚠ '${name}' is denied globally (deniedMcpServers), and Claude Code applies that in every project, project-scope entries included, so a per-project enable cannot turn it on. Enable it globally, then disable it in the projects where you do not want it: ${toolCommand("mcp-manager disable", name, "-p", "claude", "--project", "")}` + ) + ); + } // Remove from the project's disabled list (Claude Code reads this list, // so an override entry only works when the name is NOT in it) @@ -520,62 +603,66 @@ export class ClaudeProvider extends MCPProvider { } /** - * Apply a TRUE global disable to the in-memory Claude config: - * 1. Preserve each server's full config in the unified config with - * `_meta.enabled.claude = false` (import it there if missing). - * 2. Keep the top-level marker (Claude Code ignores it; mcp-manager reads it). - * 3. Remove the entry from `mcpServers` — the only mechanism Claude Code - * honors globally, including projects registered in the future. - * No project's `disabledMcpServers` list is written. Entries that could not - * be preserved are NOT removed (no data loss). + * The ~/.claude.json half of a global disable (the deny itself is in settings.json): + * 1. Record `_meta.enabled.claude = false` in the unified config (import the config there if + * it is missing). + * 2. Keep the entry in `mcpServers`; put it back from the unified config when an older + * delete-based disable removed it, unless Grok would then start it (`restoreMissing`). + * 3. Drop the names from the top-level `disabledMcpServers` marker, which Claude Code ignores. + * No project's `disabledMcpServers` list is written. + * + * Exception, `overridden`: a server that some project runs through its own project-scope entry. + * The deny matches the name in every scope and would switch those projects off too, so such a + * server keeps the older switch instead: absent from the global `mcpServers` (its config stays in + * the unified config and in the project entries), not denied. */ - private async applyGlobalDisable(serverNames: string[], config: ClaudeGenericConfig): Promise { - if (!config.disabledMcpServers) { - config.disabledMcpServers = []; - } - - const safeToRemove = await this.preserveServersInUnifiedConfig(serverNames, config); + private async applyGlobalDisable( + serverNames: string[], + config: ClaudeGenericConfig, + overridden: string[] = [] + ): Promise { + const preserved = await this.preserveServersInUnifiedConfig(serverNames, config); + await this.restoreMissing( + serverNames.filter((name) => !overridden.includes(name)), + config, + { staysOffInClaude: true } + ); + dropTopLevelMarker(config, serverNames); - for (const serverName of serverNames) { - // Top-level marker (ignored by Claude Code, kept for back-compat) - if (!config.disabledMcpServers.includes(serverName)) { - config.disabledMcpServers.push(serverName); + for (const name of overridden) { + if (config.mcpServers?.[name] && preserved.has(name)) { + delete config.mcpServers[name]; } - // TRUE global disable: remove the entry from mcpServers - if (config.mcpServers?.[serverName]) { - if (safeToRemove.has(serverName)) { - delete config.mcpServers[serverName]; - } else { - logger.warn( - chalk.yellow( - `⚠ '${serverName}' was NOT removed from mcpServers in ${this.configPath} because its config could not be preserved in the unified config, so Claude Code still loads it in every project.` - ) - ); - } - } + this.reportOverridden(name, config); } } + private reportOverridden(name: string, config: ClaudeGenericConfig): void { + logger.warn( + chalk.yellow( + `⚠ '${name}' keeps the older global switch (absent from mcpServers, not denied): ${projectsOverriding(config, name)} project(s) run it through a project-scope entry, and a deny would switch those projects off too.` + ) + ); + } + /** - * Apply a global enable to the in-memory Claude config: restore the entry - * into `mcpServers` from the unified config (a TRUE global disable removed - * it) and drop the top-level marker. Project lists stay as they are unless - * `clearProjectOverrides` asks to take the name off every list that still - * names it; without it, one warning gives the count and the flag. + * The ~/.claude.json half of a global enable (the caller then lifts the deny): put the entry + * back into `mcpServers` when an older delete-based disable removed it, and drop the top-level + * marker. Grok sees a restored entry as well; that matches the old behaviour of a global + * enable, and the server is meant to run. Project lists stay as they are unless + * `clearProjectOverrides` asks to take the name off every list that still names it; without + * it, one warning gives the count and the flag. */ private async applyGlobalEnable( serverNames: string[], config: ClaudeGenericConfig, options: EnableServersOptions = {} ): Promise { - await this.restoreServersFromUnifiedConfig(serverNames, config); + await this.restoreMissing(serverNames, config, { staysOffInClaude: false }); + dropTopLevelMarker(config, serverNames); for (const serverName of serverNames) { - if (config.disabledMcpServers) { - config.disabledMcpServers = config.disabledMcpServers.filter((name) => name !== serverName); - } - const listing = Object.values(config.projects ?? {}).filter((project) => project.disabledMcpServers?.includes(serverName) ); @@ -604,11 +691,8 @@ export class ClaudeProvider extends MCPProvider { } /** - * Ensure full server configs are preserved in the unified config with - * `_meta.enabled.claude = false` BEFORE they are removed from - * ~/.claude.json. Returns the set of server names whose config body is - * safely stored in the unified config (pre-existing or successfully - * imported) — only those may be removed without data loss. + * Record `_meta.enabled.claude = false` in the unified config, importing a server's config + * there when it is missing. Returns the names whose config the unified config now holds. */ private async preserveServersInUnifiedConfig( serverNames: string[], @@ -649,21 +733,42 @@ export class ClaudeProvider extends MCPProvider { } /** - * Restore entries removed by a TRUE global disable: copy the server config - * back into `mcpServers` from the unified config (where it was preserved). + * Put entries an older delete-based disable removed back into `mcpServers`, from the unified + * config (or from `source` when the caller has the projected config). For a server that stays + * off in Claude (`staysOffInClaude`), Grok decides: it reads `mcpServers` too and Claude's deny + * does not reach it, so the entry is put back only when Grok would not start it or the unified + * config enables it for Grok. Otherwise it stays out, and Claude's deny still covers it. */ - private async restoreServersFromUnifiedConfig(serverNames: string[], config: ClaudeGenericConfig): Promise { + private async restoreMissing( + serverNames: string[], + config: ClaudeGenericConfig, + opts: { staysOffInClaude: boolean; source?: Record } + ): Promise { const missing = serverNames.filter((name) => !config.mcpServers?.[name]); if (missing.length === 0) { return; } const unified = await readUnifiedConfig(); + const grokWouldStart = opts.staysOffInClaude + ? await namesGrokWouldStartFromClaudeJson(missing, unified) + : new Set(); + for (const name of missing) { - const entry = unified.mcpServers[name]; + const entry = opts.source?.[name] ?? unified.mcpServers[name]; if (!entry) { continue; // Not in unified config — nothing to restore } + + if (grokWouldStart.has(name) && unified.mcpServers[name]?._meta?.enabled?.grok !== true) { + logger.warn( + chalk.yellow( + `⚠ '${name}' stays out of mcpServers in ${this.configPath}: Grok loads that list, has no table for it and would start it, and Claude's deny does not reach Grok. Claude's deny still keeps it off in Claude.` + ) + ); + continue; + } + if (!config.mcpServers) { config.mcpServers = {}; } @@ -716,41 +821,66 @@ export class ClaudeProvider extends MCPProvider { } } - return this.writeConfig(config); + const written = await this.writeConfig(config); + + if (written === WriteResult.Rejected) { + return written; + } + + // A deny entry left behind would block a later server of the same name. + return combineWrites(written, await this.setDeniedServerNames(serverNames, false)); } + /** + * `_meta.enabled.claude === false`: denied in settings.json, and the config kept in (or put back + * into) `mcpServers`, Grok permitting (`restoreMissing`). `true`: in `mcpServers`, and its + * `serverName` deny entry removed. Unset or a per-project map: in `mcpServers`, deny left as it + * is. The top-level `disabledMcpServers` marker is removed: Claude Code never reads it. The + * denylist is written first; when that write is rejected, ~/.claude.json is not written either, + * so a sync never puts an entry back without its deny. + */ async syncServers(servers: Record): Promise { + const settings = await this.readSettings(); + const current = await this.readConfig(); + const disabled = Object.keys(servers).filter((name) => servers[name]._meta?.enabled?.claude === false); + // Same exception as a global disable: a server some project overrides keeps the older switch. + const overridden = disabled.filter((name) => projectsOverriding(current, name) > 0); + const deny = disabled.filter((name) => !overridden.includes(name)); + const allow = Object.keys(servers).filter((name) => servers[name]._meta?.enabled?.claude === true); + let denyResult = WriteResult.NoChanges; + + if (this.applyDenyChanges(settings, { deny, allow })) { + denyResult = await this.writeSettings(settings); + + if (denyResult === WriteResult.Rejected) { + return denyResult; + } + } + const config = await this.readConfig(); if (!config.mcpServers) { config.mcpServers = {}; } - if (!config.disabledMcpServers) { - config.disabledMcpServers = []; + + delete config.disabledMcpServers; + await this.restoreMissing(deny, config, { staysOffInClaude: true, source: servers }); + + for (const name of overridden) { + delete config.mcpServers[name]; + this.reportOverridden(name, config); } for (const [name, serverConfig] of Object.entries(servers)) { const cleanConfig = stripMeta(serverConfig); - const enabledState = serverConfig._meta?.enabled?.claude; - const isGloballyEnabled = enabledState === true; - const isGloballyDisabled = enabledState === false; - if (isGloballyDisabled) { - // TRUE global disable = absent from mcpServers (Claude Code - // never reads the top-level disabledMcpServers key, and the - // per-project sweep misses projects registered later). - delete config.mcpServers[name]; - } else { + // A disabled server keeps its config in mcpServers (the deny is the switch); one that + // restoreMissing kept out because of Grok, or an overridden one, stays out. + if (enabledState !== false || (config.mcpServers[name] && !overridden.includes(name))) { config.mcpServers[name] = this.unifiedToClaude(cleanConfig); } - if (isGloballyEnabled) { - config.disabledMcpServers = config.disabledMcpServers.filter((n) => n !== name); - } else if (!config.disabledMcpServers.includes(name)) { - config.disabledMcpServers.push(name); - } - // Per-project override entry (.projects[].mcpServers.): // refresh its config from the unified config, NEVER delete it. for (const projectConfig of Object.values(config.projects ?? {})) { @@ -766,7 +896,7 @@ export class ClaudeProvider extends MCPProvider { } } - return this.writeConfig(config); + return combineWrites(denyResult, await this.writeConfig(config)); } /** @@ -819,9 +949,8 @@ export class ClaudeProvider extends MCPProvider { * Check if a server is enabled in this provider's config */ async isServerEnabled(serverName: string): Promise { - const config = await this.readConfig(); - const disabledServers = new Set(config.disabledMcpServers || []); - return !disabledServers.has(serverName); + const url = (await this.readConfig()).mcpServers?.[serverName]?.url; + return !isDeniedByClaude(await this.readDeniedEntries(), serverName, url); } fromUnifiedConfig(servers: Record): unknown { diff --git a/src/mcp-manager/utils/providers/grok.ts b/src/mcp-manager/utils/providers/grok.ts index 01c8b3410e..7e43e3aab3 100644 --- a/src/mcp-manager/utils/providers/grok.ts +++ b/src/mcp-manager/utils/providers/grok.ts @@ -75,6 +75,52 @@ function grokConfigPathForHome(home: string): string { return path.join(expanded, "config.toml"); } +/** + * Of `names`, the ones Grok would START if they appeared in `~/.claude.json` top-level `mcpServers`: + * Grok reads that list (unless `[compat.claude] mcps = false`), Claude's `deniedMcpServers` does not + * reach Grok, a config.toml table shadows the Claude copy, and `disabled_mcp_servers` stops it. + * Checked in every Grok home the unified config names; a home without a config.toml has no Grok. + * A config.toml that does not parse counts as "would start", the safe answer for the caller. + */ +export async function namesGrokWouldStartFromClaudeJson( + names: string[], + unified: UnifiedMCPConfig +): Promise> { + const result = new Set(); + + for (const home of resolveHarnessHomes(unified, "grok", "syncTo")) { + const configPath = grokConfigPathForHome(home); + if (!existsSync(configPath)) { + continue; + } + + let config: GrokGenericConfig; + try { + config = TOML.parse(await readFile(configPath, "utf-8")) as GrokGenericConfig; + } catch (error) { + logger.warn({ error, configPath }, "mcp-manager: could not read a Grok config; assuming Grok would start"); + for (const name of names) { + result.add(name); + } + + continue; + } + + if (!compatMcpsOn(config, "claude")) { + continue; + } + + const disabled = stringList(config.disabled_mcp_servers); + for (const name of names) { + if (!config.mcp_servers?.[name] && !disabled.includes(name)) { + result.add(name); + } + } + } + + return result; +} + export class GrokProvider extends MCPProvider { constructor() { const home = path.join(env.paths.getHome() || env.paths.getUserProfile() || "~", ".grok"); From 0b01fa0de9ce5437992c470b7e9e5466693b0e67 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 13 Oct 2026 20:35:48 +0200 Subject: [PATCH 02/32] fix(ai, statusline): a narrow pane shortens the branch in the middle instead of dropping it, and lines fit the COLUMNS - 4 cells Claude Code draws (cherry picked from commit e00aceabea93957dd7d917642c907981218a7c48) --- src/utils/ai/statusline/layout.ts | 26 +++++++++++++-- src/utils/ai/statusline/render.ts | 23 +++++++++---- src/utils/ai/statusline/segments.ts | 22 +++++++++++++ src/utils/ai/statusline/statusline.test.ts | 38 +++++++++++++++++++++- 4 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/utils/ai/statusline/layout.ts b/src/utils/ai/statusline/layout.ts index 6c93d7a99f..31b2386f12 100644 --- a/src/utils/ai/statusline/layout.ts +++ b/src/utils/ai/statusline/layout.ts @@ -9,8 +9,8 @@ export function visibleWidth(text: string): number { /** * Concatenate parts left to right, skipping any part that would push the line past - * `maxWidth`. Later parts still get a chance, exactly like the shell `build_line`, so a long - * branch name drops the branch and keeps the dirty count. + * `maxWidth`. Later parts still get a chance, exactly like the shell `build_line`. The branch + * is fitted to the room before it gets here (`fitBranch`), so only a too-narrow line drops it. */ export function buildLine(parts: readonly string[], maxWidth: number): string { let result = ""; @@ -26,6 +26,28 @@ export function buildLine(parts: readonly string[], maxWidth: number): string { return result; } +/** + * Cut the middle of `text` so it is at most `max` characters and both ends stay readable: + * `feat/2026-10-13-enhancements-unified` at 20 reads `feat/2026-…s-unified`. Counts code + * points like `visibleWidth`, and returns the text unchanged when it already fits. + */ +export function truncateMiddle(text: string, max: number): string { + const chars = Array.from(text); + + if (chars.length <= max) { + return text; + } + + if (max <= 1) { + return max === 1 ? "…" : ""; + } + + const tail = Math.floor((max - 1) / 2); + const head = max - 1 - tail; + + return `${chars.slice(0, head).join("")}…${tail > 0 ? chars.slice(-tail).join("") : ""}`; +} + /** * The terminal width the host renders into. The statusline runs with piped stdio, so the * columns are read from stderr when it is still the terminal, then `COLUMNS`, then `tput`, diff --git a/src/utils/ai/statusline/render.ts b/src/utils/ai/statusline/render.ts index bd248bc17f..75a5f5cfc8 100644 --- a/src/utils/ai/statusline/render.ts +++ b/src/utils/ai/statusline/render.ts @@ -6,12 +6,13 @@ import { SafeJSON } from "@genesiscz/utils/json"; import { logger } from "@genesiscz/utils/logger"; import { killWithEscalation } from "@genesiscz/utils/process/killWithEscalation"; import { StatuslineCache } from "./cache"; -import { buildLine, terminalWidth } from "./layout"; +import { buildLine, terminalWidth, visibleWidth } from "./layout"; import { accountSegment, contextSegment, deltaSegment, dirtySegment, + fitBranch, gitSegment, modelDirSegment, modelLabel, @@ -39,6 +40,13 @@ export interface GitInfo { dirty: number; } +/** + * Claude Code draws the statusline 2 cells in from each edge and cuts a longer line with its own + * `…`: a 59-column pane showed 55 cells, a 53-column pane 49. A line built to `COLUMNS - 2` lost + * its last two cells, so the budget is `COLUMNS - 4`. + */ +const HOST_PADDING_COLUMNS = 4; + /** A hung `git status` must not freeze every Claude statusline on the machine. */ const GIT_STATUS_TIMEOUT_MS = 5_000; const SPAWN_KILL_GRACE_MS = 250; @@ -85,10 +93,14 @@ export async function renderStatusline(raw: Record, deps: Rende time("extension", async () => (config.extends ? runExtension(payload, config) : [])), ]); - const line1Parts = [ - modelDirSegment(modelLabel(model, payload.modelDisplayName, config.modelStyle), basename(payload.cwd)), - gitSegment(git?.branch ?? null), - ]; + const maxWidth = (deps.columns ?? terminalWidth(config.fallbackColumns)) - HOST_PADDING_COLUMNS; + const modelDir = modelDirSegment( + modelLabel(model, payload.modelDisplayName, config.modelStyle), + basename(payload.cwd) + ); + // The git segment is a space plus the name, so the name gets what is left after that space. + const branch = fitBranch(git?.branch ?? null, maxWidth - visibleWidth(modelDir) - 1); + const line1Parts = [modelDir, gitSegment(branch)]; const line2Parts: string[] = [config.showDirty ? dirtySegment(git?.branch ?? null, git?.dirty ?? 0) : ""]; if (payload.contextWindowSize && payload.usage) { @@ -112,7 +124,6 @@ export async function renderStatusline(raw: Record, deps: Rende line2Parts.push(accountSegment(account, now())); } - const maxWidth = (deps.columns ?? terminalWidth(config.fallbackColumns)) - 2; const line1 = buildLine(line1Parts, maxWidth); const line2 = buildLine(line2Parts, maxWidth); const lines = line2 ? [line1, line2] : [line1]; diff --git a/src/utils/ai/statusline/segments.ts b/src/utils/ai/statusline/segments.ts index 471e1869c6..6f0b07eba7 100644 --- a/src/utils/ai/statusline/segments.ts +++ b/src/utils/ai/statusline/segments.ts @@ -1,3 +1,4 @@ +import { truncateMiddle } from "./layout"; import type { AccountSegmentData } from "./types"; /** @@ -103,6 +104,27 @@ export function gitSegment(branch: string | null): string { return branch ? ` ${ANSI.magenta}${branch}${ANSI.reset}` : ""; } +/** The fewest branch characters still worth printing; with less room the line goes without it. */ +export const MIN_BRANCH_WIDTH = 8; + +/** + * The branch as it fits in the `room` characters left after the model and the dir. Claude Code + * passes its own pane width as `COLUMNS`, so a 59-column pane leaves 55, and the raw model id + * plus the dir already take 28 of them. A branch that does not fit loses its middle instead of + * vanishing whole; below `MIN_BRANCH_WIDTH` it is still dropped. + */ +export function fitBranch(branch: string | null, room: number): string | null { + if (!branch) { + return null; + } + + if (Array.from(branch).length <= room) { + return branch; + } + + return room >= MIN_BRANCH_WIDTH ? truncateMiddle(branch, room) : null; +} + export function dirtySegment(branch: string | null, dirty: number): string { return branch && dirty > 0 ? `${ANSI.cyan}*${dirty}${ANSI.reset}` : ""; } diff --git a/src/utils/ai/statusline/statusline.test.ts b/src/utils/ai/statusline/statusline.test.ts index 4fcb4eaf94..06098f8170 100644 --- a/src/utils/ai/statusline/statusline.test.ts +++ b/src/utils/ai/statusline/statusline.test.ts @@ -15,6 +15,7 @@ import { env } from "@genesiscz/utils/env"; import * as gitCore from "@genesiscz/utils/git/core"; import { TestRepo } from "@genesiscz/utils/git/test-repo"; import { SafeJSON } from "@genesiscz/utils/json"; +import { stripAnsi } from "@genesiscz/utils/string"; import { isolatedPreviewCache, isolatedPreviewCacheDir, StatuslineCache } from "./cache"; import { defaultStatuslineConfig, @@ -27,7 +28,7 @@ import { rememberPreviousCommand, saveStatuslineConfig, } from "./config"; -import { buildLine, visibleWidth } from "./layout"; +import { buildLine, truncateMiddle, visibleWidth } from "./layout"; import { gitInfo, renderStatusline } from "./render"; import { ANSI, @@ -110,6 +111,13 @@ describe("statusline segments reproduce the shell script", () => { expect(visibleWidth(`${ANSI.red}abc${ANSI.reset}`)).toBe(3); expect(buildLine(["12345", "67890", "x"], 6)).toBe("12345x"); }); + + test("truncateMiddle keeps both ends, counts code points, and leaves a fitting text alone", () => { + expect(truncateMiddle("feat/2026-10-13-enhancements-unified", 20)).toBe("feat/2026-…s-unified"); + expect(truncateMiddle("main", 20)).toBe("main"); + expect(truncateMiddle("🌿abcdef", 4)).toBe("🌿a…f"); + expect(truncateMiddle("abcdef", 1)).toBe("…"); + }); }); describe("Claude Code payload", () => { @@ -188,6 +196,34 @@ describe("renderStatusline", () => { await second.settled; }); + /** + * Claude Code passes its pane width as `COLUMNS`. In a 59-column pane the raw model id and + * the dir take 28 of the 55 usable columns, and a 36-character branch used to vanish whole. + */ + test("a narrow pane keeps the branch by cutting its middle, and drops it only below eight characters", async () => { + const root = mkdtempSync(join(tmpdir(), "statusline-narrow-")); + const cwd = join(root, "GenesisTools"); + mkdirSync(join(cwd, ".git"), { recursive: true }); + writeFileSync(join(cwd, ".git", "HEAD"), "ref: refs/heads/feat/2026-10-13-enhancements-unified\n"); + const cache = new StatuslineCache(join(root, "cache")); + const config = mergeStatuslineConfig(defaultStatuslineConfig(), { + graft: { enabled: false, shim: "", ttlMs: 0 }, + metricsPost: { enabled: false, url: "", timeoutMs: 0 }, + }); + const narrowFeature: StatuslineFeature = { ...feature, resolveModel: async () => "claude-opus-5-5" }; + const raw = { workspace: { current_dir: cwd, project_dir: cwd }, session_id: "1b4001ba-aaaa" }; + const line1 = async (columns: number) => { + const result = await renderStatusline(raw, { feature: narrowFeature, config, cache, columns }); + + return stripAnsi(result.lines[0] ?? ""); + }; + + expect(await line1(59)).toBe("claude-opus-5-5 GenesisTools feat/2026-10-…ents-unified"); + expect(visibleWidth(await line1(59))).toBe(55); + expect(await line1(120)).toBe("claude-opus-5-5 GenesisTools feat/2026-10-13-enhancements-unified"); + expect(await line1(36)).toBe("claude-opus-5-5 GenesisTools"); + }); + test("a subagent frame renders nothing but the graft line", async () => { const config = mergeStatuslineConfig(defaultStatuslineConfig(), { graft: { enabled: false, shim: "", ttlMs: 0 }, From 5e2779a6150a75a5ccc7bc81c5a228f96a0101aa Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 03:06:25 +0200 Subject: [PATCH 03/32] test(ai-spend): Claude prices are checked against Claude Code's own per-model bills from 12 real sessions, one-hour cache writes included (cherry picked from commit 4fe5e2352fa14afebca15ba7b836e2bbd9bf5000) --- .../lib/fixtures/claude-code-bills.json | 146 ++++++++++++++++++ src/ai-spend/lib/pricing.test.ts | 20 ++- 2 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 src/ai-spend/lib/fixtures/claude-code-bills.json diff --git a/src/ai-spend/lib/fixtures/claude-code-bills.json b/src/ai-spend/lib/fixtures/claude-code-bills.json new file mode 100644 index 0000000000..84ca4ae514 --- /dev/null +++ b/src/ai-spend/lib/fixtures/claude-code-bills.json @@ -0,0 +1,146 @@ +[ + { + "model": "claude-opus-5-5", + "at": "2026-10-11", + "tokens": { + "input": 238, + "output": 67818, + "cacheRead": 21249331, + "cacheWrite": 232053, + "cacheWrite1h": 232053 + }, + "claudeCodeCostUsd": 7.4636022 + }, + { + "model": "claude-opus-5-5", + "at": "2026-10-11", + "tokens": { + "input": 118, + "output": 92109, + "cacheRead": 14891726, + "cacheWrite": 296648, + "cacheWrite1h": 296648 + }, + "claudeCodeCostUsd": 7.194181199999998 + }, + { + "model": "claude-opus-5-5", + "at": "2026-10-12", + "tokens": { + "input": 16, + "output": 9398, + "cacheRead": 428029, + "cacheWrite": 76394, + "cacheWrite1h": 76394 + }, + "claudeCodeCostUsd": 0.8847818 + }, + { + "model": "claude-sonnet-5-5", + "at": "2026-10-06", + "tokens": { + "input": 180, + "output": 110981, + "cacheRead": 21509927, + "cacheWrite": 346133, + "cacheWrite1h": 346133 + }, + "claudeCodeCostUsd": 6.796687400000002 + }, + { + "model": "claude-sonnet-5-5", + "at": "2026-10-11", + "tokens": { + "input": 67, + "output": 17888, + "cacheRead": 7655905, + "cacheWrite": 753726, + "cacheWrite1h": 753726 + }, + "claudeCodeCostUsd": 3.9595085 + }, + { + "model": "claude-sonnet-5-5", + "at": "2026-10-06", + "tokens": { + "input": 37, + "output": 20214, + "cacheRead": 2353059, + "cacheWrite": 156605, + "cacheWrite1h": 156605 + }, + "claudeCodeCostUsd": 1.2992458000000002 + }, + { + "model": "claude-fable-5-1", + "at": "2026-09-15", + "tokens": { + "input": 548, + "output": 35668, + "cacheRead": 3873257, + "cacheWrite": 475608, + "cacheWrite1h": 475608 + }, + "claudeCodeCostUsd": 12.269354250000003 + }, + { + "model": "claude-fable-5-1", + "at": "2026-09-08", + "tokens": { + "input": 454, + "output": 7325, + "cacheRead": 1770609, + "cacheWrite": 128526, + "cacheWrite1h": 128526 + }, + "claudeCodeCostUsd": 3.38396225 + }, + { + "model": "claude-fable-5-1", + "at": "2026-09-15", + "tokens": { + "input": 428, + "output": 6877, + "cacheRead": 1614573, + "cacheWrite": 93271, + "cacheWrite1h": 93271 + }, + "claudeCodeCostUsd": 2.617193250000001 + }, + { + "model": "claude-sonnet-5", + "at": "2026-09-16", + "tokens": { + "input": 36, + "output": 8337, + "cacheRead": 3383028, + "cacheWrite": 214393, + "cacheWrite1h": 214393 + }, + "claudeCodeCostUsd": 1.6176196000000003 + }, + { + "model": "claude-sonnet-5", + "at": "2026-10-01", + "tokens": { + "input": 4, + "output": 1890, + "cacheRead": 203321, + "cacheWrite": 206473, + "cacheWrite1h": 206473 + }, + "claudeCodeCostUsd": 0.8854642 + }, + { + "model": "claude-sonnet-5", + "at": "2026-09-29", + "tokens": { + "input": 10, + "output": 1165, + "cacheRead": 574176, + "cacheWrite": 147605, + "cacheWrite1h": 147605 + }, + "claudeCodeCostUsd": 0.7169251999999999 + } +] diff --git a/src/ai-spend/lib/pricing.test.ts b/src/ai-spend/lib/pricing.test.ts index ac31be9d46..ed43598b2b 100644 --- a/src/ai-spend/lib/pricing.test.ts +++ b/src/ai-spend/lib/pricing.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { byId } from "@genesiscz/utils/ai/catalog"; -import { DEFAULT_PRICING, priceFor, resolvePrice } from "./pricing"; +import claudeCodeBills from "./fixtures/claude-code-bills.json"; +import { costOf, DEFAULT_PRICING, priceFor, resolvePrice } from "./pricing"; import { eventCostEstimate } from "./reports/cost"; import type { SpendEvent } from "./reports/types"; @@ -78,6 +79,23 @@ describe("pricing table", () => { }); }); +describe("Claude Code's own bills", () => { + // Real per-model token sums from 12 local Claude Code sessions (2026-09-08 to 10-12), each next to the costUSD + // Claude Code itself recorded in that session's `cost-state` line. Only rows whose requests all sit in one price + // band and one dated rate were kept, so the sum prices like the requests did. Every row's cache writes are + // one-hour writes, which is what proves the 2x-input rate. Numbers only: no session id, path or content. + test.each(claudeCodeBills.map((bill) => [bill.model, bill.at, bill] as const))( + "%s on %s costs what Claude Code billed", + (model, at, bill) => { + const entry = priceFor(model, DEFAULT_PRICING); + expect(entry).not.toBeNull(); + + const ours = costOf(bill.tokens, resolvePrice(entry!, { at: new Date(at), contextTokens: 0 })); + expect(Math.abs(ours - bill.claudeCodeCostUsd) / bill.claudeCodeCostUsd).toBeLessThan(1e-6); + } + ); +}); + describe("what a recorded cost means", () => { // 1M fresh input, 1M cached, 100K output: grok-4.7 at list is $2 + $0.50 + $0.60. const turn = (overrides: Partial = {}): SpendEvent => ({ From dd17fb6553d69f7ba0f56a435aee0f508ac64fa8 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 04:29:53 +0200 Subject: [PATCH 04/32] fix(codex): tools codex tail --events numbers each turn, counting the turns already in the log, instead of printing turn ? (cherry picked from commit 884d8e153990a352760782cc632657d40ad121ad) --- src/codex/lib/tail-events.test.ts | 24 ++++++++++++- src/codex/lib/tail-events.ts | 58 ++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/codex/lib/tail-events.test.ts b/src/codex/lib/tail-events.test.ts index 3b5b5f1464..6b17e91242 100644 --- a/src/codex/lib/tail-events.test.ts +++ b/src/codex/lib/tail-events.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { SafeJSON } from "@genesiscz/utils/json"; -import { planFollowRead, takeCompleteLines } from "./tail-events"; +import { countStartedTurns, numberTurnLine, planFollowRead, takeCompleteLines } from "./tail-events"; describe("takeCompleteLines", () => { // The regression: a size snapshot cut a record in half, the half failed to @@ -79,3 +79,25 @@ describe("planFollowRead", () => { expect(planFollowRead(0, 120)).toEqual({ from: 0, rewound: true }); }); }); + +describe("turn numbers", () => { + test("numbers a Codex turn's start and end lines, continuing after the turns already in the log", () => { + const turns = { n: 2 }; + + expect(numberTurnLine("▶ turn ? started", turns)).toBe("▶ turn 3 started"); + expect(numberTurnLine("✔ turn ? completed ($0.0100)", turns)).toBe("✔ turn 3 completed ($0.0100)"); + expect(numberTurnLine("🔧 command ls", turns)).toBe("🔧 command ls"); + expect(numberTurnLine("▶ turn 7 started", turns)).toBe("▶ turn 7 started"); + }); + + test("counts the turns a log already started", () => { + const started = SafeJSON.stringify({ source: "app-server", method: "turn/started", params: { threadId: "t" } }); + const done = SafeJSON.stringify({ + source: "app-server", + method: "turn/completed", + params: { threadId: "t", turn: {} }, + }); + + expect(countStartedTurns(`${started}\n${done}\n${started}\n`)).toBe(2); + }); +}); diff --git a/src/codex/lib/tail-events.ts b/src/codex/lib/tail-events.ts index aa938dc33c..6f4cc854f7 100644 --- a/src/codex/lib/tail-events.ts +++ b/src/codex/lib/tail-events.ts @@ -20,7 +20,54 @@ export function takeCompleteLines(buffer: string): { complete: string; rest: str return { complete: buffer.slice(0, cut + 1), rest: buffer.slice(cut + 1) }; } -export function printFollowedChunk(text: string, asEvents: boolean): void { +/** + * Codex's app-server names a turn by id, not by number, so a rendered turn line read "▶ turn ? started". The + * follower counts the turns it renders (seeded from the log it skipped) and numbers the start and end lines. + */ +export interface TurnCounter { + n: number; +} + +export function numberTurnLine(line: string, turns: TurnCounter): string { + if (line.startsWith("▶ turn ? ")) { + turns.n += 1; + return line.replace("turn ?", `turn ${turns.n}`); + } + + if (line.startsWith("✔ turn ? ") && turns.n > 0) { + return line.replace("turn ?", `turn ${turns.n}`); + } + + return line; +} + +function renderedLines(text: string): string[] { + const lines: string[] = []; + + for (const rawLine of text.split("\n")) { + if (!rawLine.trim()) { + continue; + } + + try { + const line = formatStoredEventLine(SafeJSON.parse(rawLine, { strict: true }) as StoredCodexEvent); + if (line) { + lines.push(line); + } + } catch (err) { + logger.debug({ err, rawLine: rawLine.slice(0, 200) }, "[codex:tail] skipping unparseable event line"); + } + } + + return lines; +} + +/** How many turns the skipped part of the log already started, so the first followed turn gets the next number. */ +export function countStartedTurns(text: string): number { + return renderedLines(text).filter((line) => line.startsWith("▶ turn")).length; +} + +export function printFollowedChunk(text: string, asEvents: boolean, turns: TurnCounter = { n: 0 }): void { if (!asEvents) { out.print(text); return; @@ -41,7 +88,7 @@ export function printFollowedChunk(text: string, asEvents: boolean): void { const line = formatStoredEventLine(stored); if (line) { - out.println(line); + out.println(numberTurnLine(line, turns)); } } } @@ -78,6 +125,9 @@ export async function followSessionEvents(name: string, asEvents: boolean): Prom const path = sessionEventsPath(name); let offset = existsSync(path) ? statSync(path).size : 0; let carry = ""; + const turns: TurnCounter = { + n: asEvents && offset > 0 ? countStartedTurns(await Bun.file(path).slice(0, offset).text()) : 0, + }; // A streaming decoder, not `.text()` per read: a size snapshot can cut // a multi-byte codepoint in half, and decoding each byte slice on its // own turned every such character into U+FFFD. The decoder holds the @@ -104,7 +154,7 @@ export async function followSessionEvents(name: string, asEvents: boolean): Prom const { complete, rest } = takeCompleteLines(carry + chunk); carry = rest; if (complete) { - printFollowedChunk(complete, asEvents); + printFollowedChunk(complete, asEvents, turns); } offset = size; } @@ -117,7 +167,7 @@ export async function followSessionEvents(name: string, asEvents: boolean): Prom // first, in case the file ends mid-codepoint. const tail = carry + decoder.decode(); if (tail) { - printFollowedChunk(tail, asEvents); + printFollowedChunk(tail, asEvents, turns); } return; } From 78f9f5766cc281f9850496e45e5b4fb89a7a314e Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 04:52:05 +0200 Subject: [PATCH 05/32] fix(ai, grok): a malformed Grok auth file is logged by category, never with the parser message that quotes its tokens (cherry picked from commit 3490353dca4e38eae2813ae01a7f0a138f36ec7c) --- src/utils/ai/grok/auth.test.ts | 35 ++++++++++++++++++++++++++++++++-- src/utils/ai/grok/auth.ts | 10 ++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/utils/ai/grok/auth.test.ts b/src/utils/ai/grok/auth.test.ts index 65d6ef77e7..85854d7665 100644 --- a/src/utils/ai/grok/auth.test.ts +++ b/src/utils/ai/grok/auth.test.ts @@ -1,9 +1,17 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterEach, describe, expect, it, spyOn } from "bun:test"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { SafeJSON } from "@genesiscz/utils/json"; -import { decodeJwtClaims, getActiveAuthEntry, getTokenPrefix, isTokenExpired, readAuthFile } from "./auth"; +import { logger } from "@genesiscz/utils/logger"; +import { + decodeJwtClaims, + getActiveAuthEntry, + getTokenPrefix, + isTokenExpired, + readAuthFile, + readAuthFileAsync, +} from "./auth"; const TEST_PAYLOAD = { tier: 5, @@ -49,6 +57,29 @@ describe("grok auth", () => { expect(entries.get("https://auth.x.ai::client-id")?.key).toBe("token-abc"); }); + it("a malformed auth file is logged without its token text, by both readers", async () => { + tempDir = mkdtempSync(join(tmpdir(), "grok-auth-")); + const authPath = join(tempDir, "auth.json"); + writeFileSync(authPath, '{"https://auth.x.ai::client-id": {"key": "fixture-secret-token", broken}'); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + + try { + expect(readAuthFile(authPath).size).toBe(0); + expect((await readAuthFileAsync(authPath)).size).toBe(0); + expect(warn).toHaveBeenCalledTimes(2); + // The day log serializes an Error's message, which JSON.stringify would drop: read it the same way. + const logged = warn.mock.calls + .flat() + .flatMap((arg) => (arg && typeof arg === "object" ? Object.values(arg) : [arg])) + .map((value) => (value instanceof Error ? value.message : String(value))) + .join(" "); + expect(logged).toContain(authPath); + expect(logged).not.toContain("fixture-secret-token"); + } finally { + warn.mockRestore(); + } + }); + it("selects first active auth entry", () => { const entries = new Map([ [ diff --git a/src/utils/ai/grok/auth.ts b/src/utils/ai/grok/auth.ts index 11ffe6575b..4f16a840a8 100644 --- a/src/utils/ai/grok/auth.ts +++ b/src/utils/ai/grok/auth.ts @@ -36,6 +36,12 @@ function parseAuthEntries(raw: unknown): Map { return entries; } +// A parser error quotes the malformed document, and this one holds access and refresh tokens: log a category only. +function logAuthReadFailure(authPath: string, err: unknown): void { + const reason = err instanceof SyntaxError ? "invalid JSON" : "unreadable"; + logger.warn({ authPath, reason }, "grok: failed to parse auth file"); +} + export function readAuthFile(path?: string): Map { const authPath = path ?? grokAuthPath(); @@ -46,7 +52,7 @@ export function readAuthFile(path?: string): Map { try { return parseAuthEntries(SafeJSON.parse(readFileSync(authPath, "utf-8"))); } catch (err) { - logger.warn({ err, authPath }, "grok: failed to parse auth file"); + logAuthReadFailure(authPath, err); return new Map(); } } @@ -62,7 +68,7 @@ export async function readAuthFileAsync(path?: string): Promise Date: Wed, 14 Oct 2026 05:16:07 +0200 Subject: [PATCH 06/32] fix(Clicky): the library integration tests delete their per-test preferences file, which removePersistentDomain left behind (1,541 of them) (cherry picked from commit ea1bd823d41f4cbd6665284acc27f29488e38449) --- .../Tests/GenesisKitTests/ClickyTests.swift | 103 ++++++++++-------- 1 file changed, 57 insertions(+), 46 deletions(-) diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/ClickyTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/ClickyTests.swift index 014f54ccf9..efa5e2f35e 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/ClickyTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/ClickyTests.swift @@ -426,7 +426,7 @@ final class ClickyTests: XCTestCase { func testUnreadablePreferencesDoNotEraseValidTypingHistory() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } var saved = ClickyStatistics() saved.record(keyCode: 0, release: false, at: date(12)) defaults.set(Data("{invalid preferences".utf8), forKey: "clicky.preferences.v1") @@ -450,7 +450,7 @@ final class ClickyTests: XCTestCase { func testUndecodablePreferencesAreKeptBeforeTheFirstSaveReplacesThem() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let stored = Data("{\"releaseSounds\": false, \"notifications\": tru".utf8) defaults.set(stored, forKey: "clicky.preferences.v1") let model = ClickyModel(defaults: defaults, previewOnly: true, observeSystemEvents: false) @@ -468,7 +468,7 @@ final class ClickyTests: XCTestCase { func testUndecodableLibraryListIsKeptBeforeAnAddedFolderReplacesIt() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let stored = Data("[{\"id\": \"work\", \"displayName\": \"Work sou".utf8) defaults.set(stored, forKey: "clicky.libraries.v1") let library = ClickySoundLibrary(defaults: defaults, bundled: nil) @@ -478,7 +478,7 @@ final class ClickyTests: XCTestCase { XCTAssertEqual(recovery.count, 1) XCTAssertEqual(defaults.data(forKey: try XCTUnwrap(recovery.first)), stored) - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) let reopened = ClickySoundLibrary(defaults: defaults, bundled: nil) defer { reopened.stop() } XCTAssertTrue(defaults.dictionaryRepresentation().keys.allSatisfy { !$0.contains(".undecodable.") }) @@ -488,7 +488,7 @@ final class ClickyTests: XCTestCase { func testDecodablePreferencesLeaveNoRecoveryCopy() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } var saved = ClickyPreferences() saved.releaseSounds = false defaults.set(try JSONEncoder().encode(saved), forKey: "clicky.preferences.v1") @@ -503,7 +503,7 @@ final class ClickyTests: XCTestCase { func testUnreadableHistoryIsNeverOverwrittenByActivationOrShutdown() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let original = Data("{\"presses\":12000,\"minutes\":\"unreadable\"}".utf8) defaults.set(original, forKey: "clicky.statistics.v1") let monitor = InputMonitorStub() @@ -535,7 +535,7 @@ final class ClickyTests: XCTestCase { func testRepeatedResetsKeepOnlyTheNewestRecoveryCopies() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let prefix = "clicky.statistics.recovery." defaults.set(Data("from an older build".utf8), forKey: prefix + UUID().uuidString) for round in 1...5 { @@ -554,7 +554,7 @@ final class ClickyTests: XCTestCase { func testHistoryRetryReopensRestoredDataWithoutResettingOrWritingIt() throws { let suite = "dev.genesis.clicky.persistence.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let original = Data("unreadable".utf8) defaults.set(original, forKey: "clicky.statistics.v1") let model = ClickyModel(defaults: defaults, previewOnly: true, observeSystemEvents: false) @@ -583,7 +583,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } XCTAssertFalse(model.enabled) model.activate() @@ -607,7 +607,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } model.activate() XCTAssertFalse(model.enabled) @@ -632,7 +632,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } // "Allow Input Monitoring…" in the privacy note, with Clicky off. model.openInputSettings() @@ -661,7 +661,7 @@ final class ClickyTests: XCTestCase { XCTAssertEqual(monitor.permissionDialogs, result == .permissionRequired ? 1 : 0) XCTAssertEqual(monitor.starts, 1) model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } } @@ -672,7 +672,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor, snapshot: true) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } model.activate() XCTAssertFalse(model.enabled) @@ -688,7 +688,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } model.activate() let event = try XCTUnwrap(CGEvent(source: nil)) @@ -706,7 +706,7 @@ final class ClickyTests: XCTestCase { let (model, defaults, suite) = monitorFixture(monitor) defer { model.shutdown() - defaults.removePersistentDomain(forName: suite) + disposeIntegrationDefaults(defaults, suite: suite) } model.activate() let event = try XCTUnwrap(CGEvent(source: nil)) @@ -730,7 +730,7 @@ final class ClickyTests: XCTestCase { func testSettingsRegistrationPreservesPendingRequestsAndPersistsSelection() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let appearance = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) let initial = settingsSection("general", pages: ["general"], order: 0) @@ -751,7 +751,7 @@ final class ClickyTests: XCTestCase { func testSettingsRejectDuplicatePageIDsWithoutChangingTheCatalog() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let appearance = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) let store = NativeSettingsStore( @@ -775,7 +775,7 @@ final class ClickyTests: XCTestCase { func testSettingsOrderPlacesLateRegisteredFeaturesBeforeAbout() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let appearance = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) let store = NativeSettingsStore( @@ -793,7 +793,7 @@ final class ClickyTests: XCTestCase { func testSharedAppearanceIsDurableIndependentAndMigrationDoesNotOverwrite() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let first = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) first.migrateIfNeeded(reduceMotion: true, reduceTransparency: false) @@ -819,7 +819,7 @@ final class ClickyTests: XCTestCase { func testThemeChoiceSurvivesIndependentTransparencyOverrides() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let appearance = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) appearance.theme = .gradient @@ -843,7 +843,7 @@ final class ClickyTests: XCTestCase { func testIndependentAppearanceWritesDoNotOverwriteAnotherInstancesChanges() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let first = NativeSettingsAppearance( defaults: defaults, notificationNamespace: suite, observeExternalChanges: false) let stale = NativeSettingsAppearance( @@ -863,7 +863,7 @@ final class ClickyTests: XCTestCase { func testSharedAppearanceReceivesDistributedChanges() async { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let writer = NativeSettingsAppearance(defaults: defaults, notificationNamespace: suite) let reader = NativeSettingsAppearance(defaults: UserDefaults(suiteName: suite)!, notificationNamespace: suite) let changed = expectation(description: "Distributed appearance notification") @@ -880,7 +880,7 @@ final class ClickyTests: XCTestCase { func testClickyLegacyAppearanceAndSharedStoreRemainConnected() async throws { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } var legacy = ClickyPreferences() legacy.reduceMotion = true defaults.set(try JSONEncoder().encode(legacy), forKey: "clicky.preferences.v1") @@ -905,7 +905,7 @@ final class ClickyTests: XCTestCase { func testSettingsTaxonomyRetainsOneClickyModelWithoutEnablingInput() { let suite = "dev.genesis.settings.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let model = ClickyModel(defaults: defaults, previewOnly: true) let sections = ClickySettingsPages.sections(model: model) XCTAssertEqual(sections.map(\.id), ["general", "clicky", "about"]) @@ -924,7 +924,7 @@ final class ClickyTests: XCTestCase { func testNotificationPermissionRoutesDeniedToSettingsAndRetainsActivationPreference() async { let suite = "dev.genesis.clicky.notifications.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } var authorization: UNAuthorizationStatus = .denied var requests = 0 var settingsOpens = 0 @@ -961,7 +961,7 @@ final class ClickyTests: XCTestCase { func testStaleNotificationReadCannotOverwriteCompletedPermissionRequest() async { let suite = "dev.genesis.clicky.notifications.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let started = AsyncStream.makeStream() var pending: CheckedContinuation? var first = true @@ -992,7 +992,7 @@ final class ClickyTests: XCTestCase { func testNotificationSettingsFailureIsActionable() async { let suite = "dev.genesis.clicky.notifications.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let model = ClickyModel(defaults: defaults, observeSystemEvents: false, notificationClient: NativeNotificationClient(status: { .denied }, request: { XCTFail("Denied authorization must not be requested again") @@ -1038,7 +1038,7 @@ final class ClickyTests: XCTestCase { func testPreviewCannotActivateInputAndStartsOff() { let suite = "dev.genesis.clicky.test.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let model = ClickyModel(defaults: defaults, previewOnly: true) XCTAssertFalse(model.enabled) model.activate() @@ -1544,6 +1544,7 @@ extension ClickyPackTests { return (UserDefaults(suiteName: suite)!, suite) } + @MainActor private func seedLibrary(_ defaults: UserDefaults, root: URL, count: Int = 1) throws -> [ClickyLibraryRecord] { let bookmark = try root.bookmarkData(options: [.withSecurityScope], includingResourceValuesForKeys: nil, relativeTo: nil) @@ -1560,7 +1561,7 @@ extension ClickyPackTests { let fixture = try Fixture() defer { fixture.dispose() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let library = ClickySoundLibrary(defaults: defaults, bundled: fixture.root) defer { library.stop() } XCTAssertEqual(library.records.map(\.id), [ClickySoundLibrary.bundledID]) @@ -1597,7 +1598,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [a, b], prepareStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1635,7 +1636,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [base], prepareStarted: { _ in started.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1664,7 +1665,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [a, b], prepareStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1718,7 +1719,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [a, b], prepareStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1751,7 +1752,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], catalogueStarted: { _ in started.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1779,7 +1780,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [older, newer], catalogueStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } _ = try seedLibrary(defaults, root: fixture.root) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1807,7 +1808,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], catalogueStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } _ = try seedLibrary(defaults, root: firstFixture.root) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1834,7 +1835,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], catalogueStarted: { _ in started.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let library = ClickySoundLibrary(defaults: defaults, loader: loader) await loader.holdCatalogues() let importing = Task { @MainActor in await library.register(fixture.root) } @@ -1856,7 +1857,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], catalogueStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } await loader.holdCatalogues() @@ -1885,7 +1886,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], catalogueStarted: { starts[$0 - 1].fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } _ = try seedLibrary(defaults, root: firstFixture.root, count: 31) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -1957,7 +1958,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], prepareStarted: { _ in started.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -2018,7 +2019,7 @@ extension ClickyTests { let monitor = InputMonitorStub() monitor.granted = true let (model, defaults, suite) = monitorFixture(monitor) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } model.activate() XCTAssertTrue(model.enabled) model.shutdown() @@ -2070,7 +2071,7 @@ extension ClickyTests { let (model, defaults, suite) = monitorFixture(monitor, snapshot: true) model.shutdown() defaults.set(data, forKey: "clicky.preferences.v1") - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let restored = ClickyModel(defaults: defaults, previewOnly: true, inputMonitor: monitor, observeSystemEvents: false) defer { restored.shutdown() } XCTAssertEqual(restored.preferences, original) @@ -2098,7 +2099,7 @@ extension ClickyTests { let monitor = InputMonitorStub() let suite = "dev.genesis.clicky.missing-library.\(UUID().uuidString)" let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } var saved = ClickyPreferences() saved.selectedSwitch = .honey saved.selectedPack = ClickyPackReference(libraryID: "missing-library", packID: "fixture") @@ -2134,7 +2135,7 @@ extension ClickyPackTests { let loader = GatedIntegrationLoader(packs: [pack], prepareStarted: { _ in started.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -2163,7 +2164,7 @@ extension ClickyPackTests { prepareStarted: { _ in forbidden.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } let record = try XCTUnwrap(seedLibrary(defaults, root: fixture.root).first) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -2191,7 +2192,7 @@ extension ClickyPackTests { prepareStarted: { _ in forbidden.fulfill() }) addTeardownBlock { await loader.cancelPending() } let (defaults, suite) = integrationDefaults() - defer { defaults.removePersistentDomain(forName: suite) } + defer { disposeIntegrationDefaults(defaults, suite: suite) } _ = try seedLibrary(defaults, root: fixture.root) let library = ClickySoundLibrary(defaults: defaults, loader: loader) defer { library.stop() } @@ -2385,3 +2386,13 @@ extension ClickyTests { XCTAssertEqual(stats.presses, 12002) } } + +/// `removePersistentDomain` empties a suite but leaves its plist in ~/Library/Preferences, so every run of these +/// tests used to leave one file per test behind (1,541 by 2026-10-14). Remove the file too. +@MainActor +func disposeIntegrationDefaults(_ defaults: UserDefaults, suite: String) { + defaults.removePersistentDomain(forName: suite) + let plist = FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent("Library/Preferences/\(suite).plist") + try? FileManager.default.removeItem(at: plist) +} From 7fff4e5272376004051b25dbf5ca917d861b4ca7 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 05:32:14 +0200 Subject: [PATCH 07/32] fix(tools): a handoff worker commits without asking again when its brief or the orchestrator explicitly authorizes the commit (cherry picked from commit dff6959b42107b58d38ef2eb5117ac7cd305f5b1) --- src/utils/worker/contract.test.ts | 3 +++ src/utils/worker/contract.ts | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/utils/worker/contract.test.ts b/src/utils/worker/contract.test.ts index 724ce34ece..d96ec14394 100644 --- a/src/utils/worker/contract.test.ts +++ b/src/utils/worker/contract.test.ts @@ -11,6 +11,9 @@ describe("buildWorkerContract", () => { expect(text).toContain("Honor any `Stop and report` block in your task brief literally"); expect(text).toContain("fails twice in a row, STOP"); expect(text).toContain("CHARACTER FOR CHARACTER"); + // A brief or a steer that grants "commit once" is honoured; without one the worker still asks. + expect(text).toContain("running any git commit/push or destructive command,"); + expect(text).toContain("explicitly authorizes that exact action"); for (const key of WORKER_REPORT_KEYS) { expect(text).toMatch(new RegExp(`^${key}: `, "m")); } diff --git a/src/utils/worker/contract.ts b/src/utils/worker/contract.ts index 3b43b40a7e..444ddadafd 100644 --- a/src/utils/worker/contract.ts +++ b/src/utils/worker/contract.ts @@ -90,7 +90,9 @@ function checkpointLines(): string[] { return [ "You are the receiving end of a handoff. Honor any `Stop and report` block in your task brief literally —", "stopping to ask is expected behavior, not a failure. Ask before creating files outside the declared scope,", - "adding dependencies, changing public interfaces, or running any git commit/push or destructive command.", + "adding dependencies, changing public interfaces, or running any git commit/push or destructive command,", + "unless your task brief or a later message from the orchestrator explicitly authorizes that exact action", + '(for example "commit once"): then do exactly what was authorized, no more, and do not ask again.', "Run the verification command in your brief yourself and paste its real output; never report a state you did not observe.", "If a verification command fails twice in a row, STOP and report both outputs instead of patching further.", "Copy every file path from the brief CHARACTER FOR CHARACTER; do not retype or normalize paths.", From 29cbc2122d07ffd25bb31dcae88725f3d0e0d603 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 06:36:46 +0200 Subject: [PATCH 08/32] fix(agents, skills, agents-talk): agents message takes the body as positional words too, and refuses a body given twice (cherry picked from commit 00ade134887e4e81bdb93e68ea23ca488c390803) --- src/agents/commands/message.ts | 23 ++++++++++++++++++++--- src/agents/tests/request.test.ts | 10 ++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/agents/commands/message.ts b/src/agents/commands/message.ts index 3dc8b24e36..d05760d9aa 100644 --- a/src/agents/commands/message.ts +++ b/src/agents/commands/message.ts @@ -15,7 +15,7 @@ import type { AgentRecord, MessageEvent } from "../lib/types"; const log = logger.child({ component: "agents:message" }); -interface MessageOpts { +export interface MessageOpts { from?: string; to?: string; body?: string; @@ -26,6 +26,22 @@ interface MessageOpts { session?: string; } +/** + * The body may also come as positional words (`tools agents message --to x "text"`), the way agents and request + * files keep writing it; before, those words were rejected and only the help printed. Exactly one source counts. + */ +export function withPositionalBody(words: string[], opts: MessageOpts): MessageOpts { + if (words.length === 0) { + return opts; + } + + if (opts.body !== undefined || opts.bodyFile !== undefined) { + throw new FriendlyError("the message text was given twice", "Pass it once: as words, --body or --body-file."); + } + + return { ...opts, body: words.join(" ") }; +} + function resolveBody(opts: MessageOpts): string | undefined { if (opts.bodyFile) { if (opts.body) { @@ -180,6 +196,7 @@ export function registerMessageCommand(program: Command): void { "Send a bus message to agents logged into this bus session (direct, broadcast, reply, or pure ack). " + "To reach a running Claude, Codex or Grok session that is not on the bus, use `tools message `" ) + .argument("[body...]", "Message body, the same as --body") .option("--from ", "Sender agent — name or id") .option("--to ", "Recipient agents — comma-separated names or ids; empty = broadcast") .option("--body ", "Message body (omit + --reply for pure ack)") @@ -191,7 +208,7 @@ export function registerMessageCommand(program: Command): void { .option("--meta ", "Optional JSON object") .option("--private", "Mark as private (v1: stored, not enforced)") .option("--session ", "Override session resolution") - .action(async (opts: MessageOpts) => { - await runMessage(opts); + .action(async (words: string[], opts: MessageOpts) => { + await runWithFriendlyErrors(async () => runMessageImpl(withPositionalBody(words, opts))); }); } diff --git a/src/agents/tests/request.test.ts b/src/agents/tests/request.test.ts index 2a7529e650..f1b2956c0b 100644 --- a/src/agents/tests/request.test.ts +++ b/src/agents/tests/request.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { env } from "@genesiscz/utils/env"; import { watchFileFeed } from "@genesiscz/utils/fs/file-feed-watcher"; +import { withPositionalBody } from "../commands/message"; import { appendFeed, appendMessage, FeedLogCursor, readFeed } from "../lib/feed"; import { ensureSessionDir, sessionPaths } from "../lib/paths"; import { sendRequest } from "../lib/request"; @@ -154,3 +155,12 @@ describe("agents request", () => { }); }); }); + +describe("agents message body", () => { + test("positional words are the body, and a second source is refused", () => { + expect(withPositionalBody(["hello", "there"], { to: "worker" }).body).toBe("hello there"); + expect(withPositionalBody([], { body: "kept" }).body).toBe("kept"); + expect(() => withPositionalBody(["twice"], { body: "already" })).toThrow("given twice"); + expect(() => withPositionalBody(["twice"], { bodyFile: "/tmp/x" })).toThrow("given twice"); + }); +}); From 66b4badbd87be8fd52c84d054b72a4ad5b1871e4 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 08:04:11 +0200 Subject: [PATCH 09/32] fix(tools): a worker takes a commit grant in its brief as given in advance, instead of asking once first (cherry picked from commit f8b260579108a4d4c548ca8b88085a3776cc9c69) --- src/utils/worker/contract.test.ts | 4 ++++ src/utils/worker/contract.ts | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/utils/worker/contract.test.ts b/src/utils/worker/contract.test.ts index d96ec14394..a444cb10a0 100644 --- a/src/utils/worker/contract.test.ts +++ b/src/utils/worker/contract.test.ts @@ -14,6 +14,10 @@ describe("buildWorkerContract", () => { // A brief or a steer that grants "commit once" is honoured; without one the worker still asks. expect(text).toContain("running any git commit/push or destructive command,"); expect(text).toContain("explicitly authorizes that exact action"); + // A grant in the brief is not a cue to ask once first (2026-10-14: a worker with "Commit exactly ONCE" + // in its brief still stopped for confirmation, reading "do not ask again" as "ask once"). + expect(text).toContain("given in advance: do the authorized action without stopping"); + expect(text).not.toContain("do not ask again"); for (const key of WORKER_REPORT_KEYS) { expect(text).toMatch(new RegExp(`^${key}: `, "m")); } diff --git a/src/utils/worker/contract.ts b/src/utils/worker/contract.ts index 444ddadafd..1adcbe82e5 100644 --- a/src/utils/worker/contract.ts +++ b/src/utils/worker/contract.ts @@ -92,7 +92,8 @@ function checkpointLines(): string[] { "stopping to ask is expected behavior, not a failure. Ask before creating files outside the declared scope,", "adding dependencies, changing public interfaces, or running any git commit/push or destructive command,", "unless your task brief or a later message from the orchestrator explicitly authorizes that exact action", - '(for example "commit once"): then do exactly what was authorized, no more, and do not ask again.', + '(for example "commit once"). That authorization is given in advance: do the authorized action without stopping', + "to ask for it first, exactly as authorized and no more.", "Run the verification command in your brief yourself and paste its real output; never report a state you did not observe.", "If a verification command fails twice in a row, STOP and report both outputs instead of patching further.", "Copy every file path from the brief CHARACTER FOR CHARACTER; do not retype or normalize paths.", From 4476e7f8ada69a5f79661e81aa8be81053bd49b3 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 08:55:16 +0200 Subject: [PATCH 10/32] fix(codex): a worker's app-server error shows its nested message and a dropped stream's status, not unknown codex error (cherry picked from commit f46deb17bf8a75b76b00b95376b4c366b062eadd) --- src/utils/codex/worker-stream.test.ts | 25 +++++++++++++++++++++++++ src/utils/codex/worker-stream.ts | 15 ++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/utils/codex/worker-stream.test.ts diff --git a/src/utils/codex/worker-stream.test.ts b/src/utils/codex/worker-stream.test.ts new file mode 100644 index 0000000000..98fdce6776 --- /dev/null +++ b/src/utils/codex/worker-stream.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from "bun:test"; +import { toWorkerEvent } from "./worker-stream"; + +describe("codex worker stream", () => { + test("an app-server error shows its nested message and the dropped stream's status", () => { + const event = toWorkerEvent({ + source: "app-server", + method: "error", + params: { + threadId: "thread-1", + error: { + message: "Reconnecting... 3/5", + codexErrorInfo: { responseStreamDisconnected: { httpStatusCode: 503 } }, + }, + }, + }); + expect(event).toMatchObject({ kind: "error", message: "Reconnecting... 3/5 (stream dropped, HTTP 503)" }); + expect(toWorkerEvent({ source: "app-server", method: "error", params: { message: "flat" } })).toMatchObject({ + message: "flat", + }); + expect(toWorkerEvent({ source: "app-server", method: "error", params: {} })).toMatchObject({ + message: "unknown codex error", + }); + }); +}); diff --git a/src/utils/codex/worker-stream.ts b/src/utils/codex/worker-stream.ts index 98f0e1f01d..feb5481f7d 100644 --- a/src/utils/codex/worker-stream.ts +++ b/src/utils/codex/worker-stream.ts @@ -90,6 +90,19 @@ function toolCallOf(item: CodexItem, sessionId: string, ts?: string): WorkerEven * echoes); unknown methods are also null but logged at debug so a protocol * addition never disappears silently. */ +/** + * The app-server nests its text as `params.error.message` (a 503 stream drop arrives as "Reconnecting... 3/5"), so + * reading `params.message` alone printed "unknown codex error" for every one, retries included (2026-10-14). + */ +function errorMessageOf(params: Record | undefined): string { + const nested = params?.error as + | { message?: unknown; codexErrorInfo?: { responseStreamDisconnected?: { httpStatusCode?: unknown } } } + | undefined; + const message = str(params?.message) ?? str(nested?.message) ?? "unknown codex error"; + const status = nested?.codexErrorInfo?.responseStreamDisconnected?.httpStatusCode; + return typeof status === "number" ? `${message} (stream dropped, HTTP ${status})` : message; +} + export function toWorkerEvent(event: StoredCodexEvent): WorkerEvent | null { const { method, ts } = event; const params = paramsOf(event); @@ -179,7 +192,7 @@ export function toWorkerEvent(event: StoredCodexEvent): WorkerEvent | null { } } case "error": - return { kind: "error", sessionId, message: str(params?.message) ?? "unknown codex error", ts }; + return { kind: "error", sessionId, message: errorMessageOf(params), ts }; case "thread/status/changed": { const status = params?.status as { type?: string } | undefined; return status?.type === "closed" ? { kind: "session.closed", sessionId, ts } : null; From ce829f8b4cd3380e7b0c1cb4bdeb89e9106a2d4c Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 14 Oct 2026 16:49:40 +0200 Subject: [PATCH 11/32] perf(GenesisKit, diff): the file tree's git marks are built off the main thread and walk folders by UTF-8 index, so a vault status refresh no longer holds the viewer about 250 ms (cherry picked from commit 9c81fd0329c3e10a5518bdd8bf0e4c0b13d44af6) --- .../GenesisKit/Diff/Model/GitState.swift | 33 +++++++++++++------ .../GenesisKit/Diff/Source/GitRepos.swift | 26 +++++++++++---- .../GenesisKitTests/Diff/GitReposTests.swift | 23 +++++++++++++ 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Diff/Model/GitState.swift b/src/macos/GenesisKit/Sources/GenesisKit/Diff/Model/GitState.swift index c815568f70..b83e79d324 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Diff/Model/GitState.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Diff/Model/GitState.swift @@ -177,31 +177,44 @@ public struct GitRepoStatus: Equatable, Sendable { let given = treeRoot.standardizedFileURL.path let givenReal = GitWorkingTreeSource.realPath(given) let rootReal = GitWorkingTreeSource.realPath(root.standardizedFileURL.path) + let inside = givenReal + "/" + // Lengths once, and the folder walks cut at the last "/" in UTF-8: `String.count` walks every character, and + // the walks asked it at every level of every entry (117 ms per call for a vault folder with 1,825 entries, + // debug build, 2026-10-14). + let realLength = givenReal.count + let givenBytes = given.utf8.count var files: [String: GitChangeKind] = [:] var folders = Set() var counts: [String: GitLineCount] = [:] for entry in entries { guard let kind = entry.shown else { continue } let real = rootReal + "/" + entry.path - guard real.hasPrefix(givenReal + "/") else { continue } - let spelled = given + real.dropFirst(givenReal.count) + guard real.hasPrefix(inside) else { continue } + let spelled = given + real.dropFirst(realLength) files[spelled] = kind - var folder = (spelled as NSString).deletingLastPathComponent - while folder.count > given.count, folders.insert(folder).inserted { - folder = (folder as NSString).deletingLastPathComponent + var folder = Self.parent(of: spelled[...]) + while let current = folder, current.utf8.count > givenBytes, folders.insert(String(current)).inserted { + folder = Self.parent(of: current) } guard let lines = numstat[entry.path] else { continue } counts[spelled] = lines - var parent = (spelled as NSString).deletingLastPathComponent - while parent.count > given.count { - let sum = counts[parent] ?? GitLineCount(additions: 0, deletions: 0) - counts[parent] = GitLineCount(additions: sum.additions + lines.additions, deletions: sum.deletions + lines.deletions) - parent = (parent as NSString).deletingLastPathComponent + var parent = Self.parent(of: spelled[...]) + while let current = parent, current.utf8.count > givenBytes { + let key = String(current) + let sum = counts[key] ?? GitLineCount(additions: 0, deletions: 0) + counts[key] = GitLineCount(additions: sum.additions + lines.additions, deletions: sum.deletions + lines.deletions) + parent = Self.parent(of: current) } } return GitTreeDecorations(files: files, folders: folders, counts: counts) } + /// Everything before the last "/" (an absolute path's folder), nil when there is none. + private static func parent(of path: Substring) -> Substring? { + guard let slash = path.utf8.lastIndex(of: UInt8(ascii: "/")) else { return nil } + return path[.. Date: Wed, 14 Oct 2026 22:55:51 +0200 Subject: [PATCH 12/32] fix(claude): tools claude info reads account names only, because loading every secret for the statusline read the vault key from the keychain and macOS asked the user whether gt-claude may use it (cherry picked from commit 14b1182d12537ac39c35d3987f1e55f0b480a759) --- src/claude/commands/info.test.ts | 46 ++++++++++++++++++++++++++++++++ src/claude/commands/info.ts | 45 +++++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 src/claude/commands/info.test.ts diff --git a/src/claude/commands/info.test.ts b/src/claude/commands/info.test.ts new file mode 100644 index 0000000000..ee6e755d36 --- /dev/null +++ b/src/claude/commands/info.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from "bun:test"; +import { resolveInfoAccount } from "./info"; + +const vaultRead = async (): Promise => { + throw new Error("the vault was opened: the keychain would ask the user for gt-claude"); +}; + +describe("tools claude info: which account", () => { + test("a named account reads names only and never opens the vault", async () => { + const account = await resolveInfoAccount({ + name: "work", + readAccounts: async () => [{ name: "personal" }, { name: "work", label: "max 20x" }], + loadSecrets: vaultRead, + readToken: async () => "tok-work", + }); + + expect(account).toEqual({ name: "work", label: "max 20x" }); + }); + + test("a token on stdin is matched against the stored tokens", async () => { + const account = await resolveInfoAccount({ + name: undefined, + readAccounts: vaultRead, + loadSecrets: async () => ({ + getAccountsByProvider: () => [ + { name: "personal", provider: "anthropic-sub", tokens: { longLivedToken: "tok-personal" } }, + { name: "work", provider: "anthropic-sub", tokens: { longLivedToken: "tok-work" } }, + ], + }), + readToken: async () => "tok-work", + }); + + expect(account?.name).toBe("work"); + }); + + test("no name and no token reads nothing", async () => { + const account = await resolveInfoAccount({ + name: undefined, + readAccounts: vaultRead, + loadSecrets: vaultRead, + readToken: async () => null, + }); + + expect(account).toBeNull(); + }); +}); diff --git a/src/claude/commands/info.ts b/src/claude/commands/info.ts index b5edadb906..060b60e259 100644 --- a/src/claude/commands/info.ts +++ b/src/claude/commands/info.ts @@ -1,4 +1,5 @@ import { AIConfig } from "@genesiscz/utils/ai/AIConfig"; +import { AiConfigStore } from "@genesiscz/utils/ai/config/AiConfigStore"; import { toolCommand } from "@genesiscz/utils/cli/tool-command"; import type { AIAccountEntry } from "@genesiscz/utils/config/ai.types"; import { SafeJSON } from "@genesiscz/utils/json"; @@ -34,22 +35,51 @@ async function readStdinToken(): Promise { return token.length > 0 ? token : null; } -async function resolveAccount(name: string | undefined, aiConfig: AIConfig): Promise { - const accounts = aiConfig.getAccountsByProvider("anthropic-sub"); +/** What the payload needs of an account: its name and label, no secret. */ +export interface InfoAccount { + name: string; + label?: string; +} + +interface SecretAccounts { + getAccountsByProvider(provider: "anthropic-sub"): AIAccountEntry[]; +} +/** + * The statusline runs this in every Claude session. With a name it reads the config's account list only: + * `AIConfig.load()` decrypts every account's secrets, which reads the vault's master key from the keychain, and + * macOS then asks the user whether `gt-claude` may use it. Only a token on stdin needs the stored tokens to compare. + */ +export async function resolveInfoAccount({ + name, + readAccounts = readAnthropicAccounts, + loadSecrets = () => AIConfig.load(), + readToken = readStdinToken, +}: { + name: string | undefined; + readAccounts?: () => Promise; + loadSecrets?: () => Promise; + readToken?: () => Promise; +}): Promise { if (name) { - return accounts.find((a) => a.name === name) ?? null; + return (await readAccounts()).find((a) => a.name === name) ?? null; } - const token = await readStdinToken(); + const token = await readToken(); if (!token) { return null; } - return accounts.find((a) => a.tokens.longLivedToken === token) ?? null; + const config = await loadSecrets(); + return config.getAccountsByProvider("anthropic-sub").find((a) => a.tokens.longLivedToken === token) ?? null; +} + +async function readAnthropicAccounts(): Promise { + const store = await AiConfigStore.readOnly(); + return store.data().accounts.filter((a) => a.provider === "anthropic-sub"); } -async function buildPayload(account: AIAccountEntry): Promise { +async function buildPayload(account: InfoAccount): Promise { // Passive cache read only — `tools claude info` must never hit the network // (it is called from the statusline on every cache-miss refresh). const cached = await peekSharedUsage(); @@ -105,8 +135,7 @@ export function registerInfoCommand(program: Command): void { process.exit(1); } - const aiConfig = await AIConfig.load(); - const account = await resolveAccount(name, aiConfig); + const account = await resolveInfoAccount({ name }); if (!account) { if (name) { From 9a61ad8f0cc69621442bcd61ca97e628ae4d0bc1 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 15 Oct 2026 00:24:29 +0200 Subject: [PATCH 13/32] feat(ai, macos): with GenesisTools.app installed, a tool gets the vault key through the app's approval window, which shows why, what, who and which session from the kernel, because every gt- link raised its own keychain dialog with nothing to judge (cherry picked from commit 46c36d5051a94ebed012e1e6591ff982f8739b1d) --- src/ai/commands/gate/index.ts | 27 +- src/ai/commands/usage/poll-daemon.ts | 2 + src/claude/commands/info.ts | 6 +- src/macos/GenesisTools/Sources/Gate.swift | 4 +- src/macos/GenesisTools/Sources/Notify.swift | 9 +- .../GenesisTools/Sources/VaultUnlock.swift | 450 ++++++++++++++++++ .../GenesisTools/Tests/VaultUnlockTests.swift | 100 ++++ src/say/lib/credential.ts | 7 +- src/utils/ai/gate/grants.ts | 52 +- src/utils/ai/gate/index.ts | 3 + src/utils/ai/gate/types.ts | 15 + src/utils/macos/genesis-app-rpc.ts | 50 ++ src/utils/security/MasterKey.ts | 11 +- src/utils/security/SecretStore.ts | 4 + src/utils/security/keyring/app-gate.test.ts | 148 ++++++ src/utils/security/keyring/app-gate.ts | 148 ++++++ src/utils/security/keyring/types.ts | 2 +- src/utils/security/vault-purpose.ts | 69 +++ 18 files changed, 1093 insertions(+), 14 deletions(-) create mode 100644 src/macos/GenesisTools/Sources/VaultUnlock.swift create mode 100644 src/macos/GenesisTools/Tests/VaultUnlockTests.swift create mode 100644 src/utils/security/keyring/app-gate.test.ts create mode 100644 src/utils/security/keyring/app-gate.ts create mode 100644 src/utils/security/vault-purpose.ts diff --git a/src/ai/commands/gate/index.ts b/src/ai/commands/gate/index.ts index 8234db2acb..5fbe3481fd 100644 --- a/src/ai/commands/gate/index.ts +++ b/src/ai/commands/gate/index.ts @@ -8,9 +8,12 @@ import { grantsPath, isGateProvider, listGrants, + listVaultGrants, readAuditTail, requestAccountAccess, revokeGrants, + revokeVaultGrants, + vaultGrantsPath, } from "@genesiscz/utils/ai/gate"; import { suggestCommand, suggestEnumFlag } from "@genesiscz/utils/cli"; import { toolCommand } from "@genesiscz/utils/cli/tool-command"; @@ -135,9 +138,10 @@ export function registerGateCommands(program: Command): void { .option("--json", "Machine-readable output") .action((opts: { json?: boolean }) => { const grants = listGrants(); + const vault = listVaultGrants(); if (opts.json) { - out.result(grants); + out.result({ grants, vault }); return; } @@ -156,8 +160,24 @@ export function registerGateCommands(program: Command): void { } out.println(table.toString()); + + renderCliHeader("Vault key", `${vault.length} remembered by GenesisTools.app`); + const vaultTable = createBoxTable(["TOOL", "EXECUTABLE", "SCRIPT", "UNTIL", "BY"]); + + for (const grant of vault) { + vaultTable.push([ + truncateDisplay(grant.tool, 16), + truncateDisplay(grant.executable, 32), + truncateDisplay(grant.script ?? "", 40), + grant.until === null ? "always" : new Date(grant.until).toLocaleString("sv-SE").slice(0, 16), + grant.method, + ]); + } + + out.println(vaultTable.toString()); renderCliSection("Files"); out.println(` ${pc.dim(grantsPath())}`); + out.println(` ${pc.dim(vaultGrantsPath())}`); }); gate.command("revoke [client]") @@ -174,7 +194,10 @@ export function registerGateCommands(program: Command): void { } const removed = await revokeGrants(selector); - out.log.info(`Revoked ${removed} grant${removed === 1 ? "" : "s"}.`); + const vault = await revokeVaultGrants(selector); + out.log.info( + `Revoked ${removed} grant${removed === 1 ? "" : "s"} and ${vault} vault-key approval${vault === 1 ? "" : "s"}.` + ); }); gate.command("audit") diff --git a/src/ai/commands/usage/poll-daemon.ts b/src/ai/commands/usage/poll-daemon.ts index a871174a2a..eb171c7d49 100644 --- a/src/ai/commands/usage/poll-daemon.ts +++ b/src/ai/commands/usage/poll-daemon.ts @@ -17,6 +17,7 @@ import { SafeJSON } from "@genesiscz/utils/json"; import { logger, out } from "@genesiscz/utils/logger"; import { isObject } from "@genesiscz/utils/object"; import { cpuMeta, profiler } from "@genesiscz/utils/profile"; +import { setVaultPurpose } from "@genesiscz/utils/security/vault-purpose"; const ANTHROPIC_SUB = "anthropic-sub"; /** The daemon runner kills this task at 60 s (`timeoutMs` in the task log); the tick keeps 10 s of headroom. */ @@ -161,6 +162,7 @@ async function main(): Promise { beforeMain: `${Math.round((beforeMain.user + beforeMain.system) / 1000)}ms`, })); logger.info("[ai-usage] daemon poll starting"); + setVaultPurpose("refresh the usage of every AI account (the usage daemon, once a minute in the background)"); const dashConfig = await loadDashboardConfig(); const db = new UsageLimitsDb(); diff --git a/src/claude/commands/info.ts b/src/claude/commands/info.ts index 060b60e259..3c4bd991d4 100644 --- a/src/claude/commands/info.ts +++ b/src/claude/commands/info.ts @@ -4,6 +4,7 @@ import { toolCommand } from "@genesiscz/utils/cli/tool-command"; import type { AIAccountEntry } from "@genesiscz/utils/config/ai.types"; import { SafeJSON } from "@genesiscz/utils/json"; import { out } from "@genesiscz/utils/logger"; +import { withVaultPurpose } from "@genesiscz/utils/security/vault-purpose"; import type { Command } from "commander"; import pc from "picocolors"; import { normalizeLimits } from "../lib/usage/limits"; @@ -70,7 +71,10 @@ export async function resolveInfoAccount({ return null; } - const config = await loadSecrets(); + const config = await withVaultPurpose( + "match this Claude session's token to a configured account, for the statusline", + () => loadSecrets() + ); return config.getAccountsByProvider("anthropic-sub").find((a) => a.tokens.longLivedToken === token) ?? null; } diff --git a/src/macos/GenesisTools/Sources/Gate.swift b/src/macos/GenesisTools/Sources/Gate.swift index 100b3a6ddf..be5acc383b 100644 --- a/src/macos/GenesisTools/Sources/Gate.swift +++ b/src/macos/GenesisTools/Sources/Gate.swift @@ -145,8 +145,8 @@ private func gateEmit(decision: String, rememberSeconds: Int, method: String, re } /// Touch ID first, then the account password when biometrics are unavailable or the user picks -/// the fallback. A cancel at either step is a deny. -private func authenticate(reason: String, completion: @escaping (_ method: String?, _ failure: String) -> Void) { +/// the fallback. A cancel at either step is a deny. The vault window (VaultUnlock.swift) confirms with it too. +func authenticate(reason: String, completion: @escaping (_ method: String?, _ failure: String) -> Void) { let biometric = LAContext() biometric.localizedFallbackTitle = "Use Password" var unavailable: NSError? diff --git a/src/macos/GenesisTools/Sources/Notify.swift b/src/macos/GenesisTools/Sources/Notify.swift index 70df81205e..a2f2a9a22f 100644 --- a/src/macos/GenesisTools/Sources/Notify.swift +++ b/src/macos/GenesisTools/Sources/Notify.swift @@ -815,7 +815,7 @@ private func list() { /// bump it: a client discovers those from `rpc.hello`'s method list instead. let rpcProtocolVersion = 1 -let rpcMethods = ["rpc.hello", "notify.post", "notify.remove", "notify.list", "notify.status", "notify.authorize", "notify.settings", "notify.reply", "gate.approve"] +let rpcMethods = ["rpc.hello", "notify.post", "notify.remove", "notify.list", "notify.status", "notify.authorize", "notify.settings", "notify.reply", "gate.approve", "vault.unlock"] /// `GenesisTools --rpc ''`, or `--rpc -` to read the request from stdin: run one method and /// exit with one JSON line on stdout. @@ -905,6 +905,13 @@ func runRpc(_ arguments: [String]) -> Never { approveGate(request.params) + case "vault.unlock": + guard let request = try? decoder.decode(ParamsEnvelope.self, from: data) else { + emitError(code: "params_invalid", message: "vault.unlock needs params.tool and params.purpose", exitCode: 64) + } + + unlockVault(request.params) + default: emitError(code: "method_unknown", message: "unknown method \(envelope.method)", exitCode: 69) } diff --git a/src/macos/GenesisTools/Sources/VaultUnlock.swift b/src/macos/GenesisTools/Sources/VaultUnlock.swift new file mode 100644 index 0000000000..c2c447f03e --- /dev/null +++ b/src/macos/GenesisTools/Sources/VaultUnlock.swift @@ -0,0 +1,450 @@ +// The vault key's approval window (`--rpc {"method":"vault.unlock"}`), Martin's DECISION 131 b (2026-10-15): +// "App approval window, but it probably should have enough context so i know exactly what / who / which session / +// triggered that and why (each caller should give that context possibly)". +// +// A `gt-` process needs the GenesisTools vault master key. Before this, each tool's own bun link read the +// keychain item and macOS asked "gt-claude wants to use your confidential information" with nothing to judge. Now the +// tool starts this process and the app reads the item as itself (macOS asks once, naming GenesisTools). +// +// WHO comes from the kernel, never from the request: this process is the caller's child, so `getppid()` is the caller, +// and its executable, arguments, environment (the session ids), working folder and parent chain are read from there. +// WHY and WHAT come from the request, because only the caller knows them. A remembered approval for the same tool, +// executable and script skips the window; otherwise Touch ID confirms an allow. Every outcome goes to the gate's audit +// log. A deny, a cancel, a failed authentication and a timeout all reply "deny" with a reason. + +import AppKit +import Darwin +import Foundation +import Security + +struct VaultUnlockParams: Decodable { + var tool: String + var purpose: String + var secret: String? + var command: String? +} + +/// `KERN_PROCARGS2` of a process: the executable path, then `argc` arguments, then the environment. +struct ProcArgs: Equatable { + var executable: String + var arguments: [String] + var environment: [String] + + func environmentValue(_ name: String) -> String? { + let prefix = "\(name)=" + return environment.first { $0.hasPrefix(prefix) }.map { String($0.dropFirst(prefix.count)) } + } +} + +/// What the kernel says about one process. +struct VaultCaller { + var pid: pid_t + var executable: String? + var args: ProcArgs? + var cwd: String? + var ancestors: [(pid: pid_t, command: String)] + + /// The script an interpreter runs (`bun …/src/claude/index.ts`), part of the identity a grant is remembered for. + var script: String? { + guard let arguments = args?.arguments, arguments.count > 1 else { return nil } + let candidate = arguments.dropFirst().first { !$0.hasPrefix("-") } + guard let candidate, [".ts", ".tsx", ".js", ".mjs", ".cjs"].contains(where: candidate.hasSuffix) else { return nil } + return candidate + } + + /// Flags and environment that load code the script path does not name. Any of them: no remembered approval. + var injected: [String] { + var found: [String] = [] + let environment = args?.environmentValue + if let value = environment?("DYLD_INSERT_LIBRARIES"), !value.isEmpty { + found.append("DYLD_INSERT_LIBRARIES") + } + + for name in ["NODE_OPTIONS", "BUN_OPTIONS"] { + if let value = environment?(name), ["--require", "-r ", "--import", "--loader", "--preload"].contains(where: value.contains) { + found.append(name) + } + } + + let flags = (args?.arguments.dropFirst() ?? []).prefix { $0.hasPrefix("-") } + if flags.contains(where: { ["--preload", "-r", "--require", "--import", "--loader"].contains($0) }) { + found.append("interpreter flags \(flags.joined(separator: " "))") + } + + return found + } +} + +enum VaultProcess { + /// Little-endian `argc`, the executable path, NUL padding, `argc` arguments, the environment up to an empty entry. + static func parseProcArgs(_ bytes: [UInt8]) -> ProcArgs? { + guard bytes.count > 4 else { return nil } + let argc = Int(bytes[0]) | Int(bytes[1]) << 8 | Int(bytes[2]) << 16 | Int(bytes[3]) << 24 + var index = 4 + + func next() -> String? { + guard index < bytes.count else { return nil } + let start = index + while index < bytes.count, bytes[index] != 0 { + index += 1 + } + + let value = String(decoding: bytes[start.. ProcArgs? { + var mib: [Int32] = [CTL_KERN, KERN_PROCARGS2, pid] + var size = 0 + guard sysctl(&mib, 3, nil, &size, nil, 0) == 0, size > 0 else { return nil } + var buffer = [UInt8](repeating: 0, count: size) + guard sysctl(&mib, 3, &buffer, &size, nil, 0) == 0 else { return nil } + return parseProcArgs(Array(buffer.prefix(size))) + } + + static func path(_ pid: pid_t) -> String? { + var buffer = [CChar](repeating: 0, count: 4 * Int(MAXPATHLEN)) + guard proc_pidpath(pid, &buffer, UInt32(buffer.count)) > 0 else { return nil } + return String(cString: buffer) + } + + static func cwd(_ pid: pid_t) -> String? { + var info = proc_vnodepathinfo() + let size = proc_pidinfo(pid, PROC_PIDVNODEPATHINFO, 0, &info, Int32(MemoryLayout.size)) + guard size > 0 else { return nil } + return withUnsafePointer(to: info.pvi_cdir.vip_path) { + String(cString: UnsafeRawPointer($0).assumingMemoryBound(to: CChar.self)) + } + } + + static func parent(_ pid: pid_t) -> pid_t? { + var info = proc_bsdinfo() + let size = proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, &info, Int32(MemoryLayout.size)) + guard size > 0, info.pbi_ppid > 0 else { return nil } + return pid_t(info.pbi_ppid) + } + + /// `name first-argument` of a process, short enough for one link of the chain. + static func label(_ pid: pid_t) -> String { + let name = path(pid).map { ($0 as NSString).lastPathComponent } ?? "pid \(pid)" + let first = procArgs(pid)?.arguments.dropFirst().first { !$0.hasPrefix("-") }.map { ($0 as NSString).lastPathComponent } + let text = [name, first].compactMap { $0 }.joined(separator: " ") + return text.count > 60 ? String(text.prefix(59)) + "…" : text + } + + static func caller(_ pid: pid_t) -> VaultCaller { + var ancestors: [(pid: pid_t, command: String)] = [] + var current = parent(pid) + while let ancestor = current, ancestor > 1, ancestors.count < 8 { + ancestors.append((ancestor, label(ancestor))) + current = parent(ancestor) + } + + return VaultCaller(pid: pid, executable: path(pid), args: procArgs(pid), cwd: cwd(pid), ancestors: ancestors) + } +} + +/// The session the caller belongs to, from its own environment and its parent chain. +enum VaultSession { + static func describe(_ caller: VaultCaller) -> String? { + var parts: [String] = [] + if let id = caller.args?.environmentValue("CLAUDE_CODE_SESSION_ID"), !id.isEmpty { + parts.append("Claude Code session \(id)") + } else if let agent = caller.ancestors.first(where: { ["claude", "codex", "grok"].contains($0.command.split(separator: " ").first.map(String.init) ?? "") }) { + parts.append("\(agent.command) (pid \(agent.pid))") + } + + if let tab = caller.args?.environmentValue("CMUX_SURFACE_ID"), !tab.isEmpty { + parts.append("cmux tab \(tab)") + } + + return parts.isEmpty ? nil : parts.joined(separator: ", ") + } +} + +/// One remembered approval: this tool, this executable, this script. +struct VaultGrant: Codable, Equatable { + var key: String + var tool: String + var executable: String + var script: String? + var grantedAt: Int64 + /// Epoch milliseconds; nil means "always", until revoked with `tools ai gate revoke`. + var until: Int64? + var method: String + + func isLive(at now: Int64) -> Bool { + until.map { $0 > now } ?? true + } +} + +struct VaultGrantsFile: Codable { + var version: Int + var grants: [VaultGrant] +} + +enum VaultGrants { + static func key(tool: String, executable: String, script: String?) -> String { + "vault|\(tool)|\(executable)|\(script ?? "")" + } + + static var directory: String { + (genesisHome() as NSString).appendingPathComponent(".genesis-tools/ai/gate") + } + + static var path: String { + (directory as NSString).appendingPathComponent("vault-grants.json") + } + + static func load(from path: String = path) -> VaultGrantsFile { + guard let data = FileManager.default.contents(atPath: path), + let file = try? JSONDecoder().decode(VaultGrantsFile.self, from: data) else { + return VaultGrantsFile(version: 1, grants: []) + } + + return file + } + + static func live(_ key: String, now: Int64, in file: VaultGrantsFile) -> VaultGrant? { + file.grants.last { $0.key == key && $0.isLive(at: now) } + } + + /// Replaces any grant for the same key and drops expired ones. + static func remember(_ grant: VaultGrant, now: Int64, in file: VaultGrantsFile) -> VaultGrantsFile { + var next = file + next.grants = file.grants.filter { $0.key != grant.key && $0.isLive(at: now) } + [grant] + return next + } + + static func save(_ file: VaultGrantsFile, to path: String = path) { + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + guard let data = try? encoder.encode(file) else { return } + try? FileManager.default.createDirectory(atPath: (path as NSString).deletingLastPathComponent, withIntermediateDirectories: true) + let temporary = path + ".tmp-\(getpid())" + guard FileManager.default.createFile(atPath: temporary, contents: data, attributes: [.posixPermissions: 0o600]) else { return } + _ = rename(temporary, path) + } +} + +/// One open window per tool: the usage daemon starts a process every minute, and without this each one would open its +/// own window while the first waits for an answer. The lock lives as long as this process (one request, one window). +enum VaultWindowLock { + static func path(for key: String) -> String { + let hash = key.utf8.reduce(UInt64(14_695_981_039_346_656_037)) { ($0 ^ UInt64($1)) &* 1_099_511_628_211 } + return (VaultGrants.directory as NSString).appendingPathComponent("vault-window-\(String(hash, radix: 16)).lock") + } + + /// The descriptor holding the lock, or nil when another process holds it. Never closed: the process exits with it. + static func acquire(_ path: String) -> Int32? { + try? FileManager.default.createDirectory(atPath: (path as NSString).deletingLastPathComponent, withIntermediateDirectories: true) + let fd = open(path, O_CREAT | O_RDWR, 0o600) + guard fd >= 0 else { return nil } + guard flock(fd, LOCK_EX | LOCK_NB) == 0 else { + close(fd) + return nil + } + + return fd + } +} + +enum VaultWindow { + static func title(_ params: VaultUnlockParams) -> String { + "“\(params.tool)” asks for the GenesisTools vault key" + } + + static func body(_ params: VaultUnlockParams, caller: VaultCaller, session: String?) -> String { + var lines: [String] = ["Why: \(params.purpose)"] + if let secret = params.secret, !secret.isEmpty { + lines.append("It was reading: \(secret)") + } + + lines.append("") + lines.append("Who (read from the system, not from the request):") + lines.append("Process: pid \(caller.pid)") + lines.append("Executable: \(caller.executable ?? "unknown")") + if let script = caller.script { + lines.append("Script: \(script)") + } + + if let arguments = caller.args?.arguments, !arguments.isEmpty { + let command = arguments.joined(separator: " ") + lines.append("Command: \(command.count > 300 ? String(command.prefix(299)) + "…" : command)") + } + + if let cwd = caller.cwd { + lines.append("Working dir: \(cwd)") + } + + if !caller.ancestors.isEmpty { + lines.append("Started by: " + caller.ancestors.map { "\($0.command) (\($0.pid))" }.joined(separator: " ← ")) + } + + lines.append("Session: \(session ?? "none found (not started from an agent session)")") + if !caller.injected.isEmpty { + lines.append("⚠️ Extra code may be loaded: \(caller.injected.joined(separator: ", ")). Allow once at most.") + } + + lines.append("") + lines.append("The vault key decrypts every secret GenesisTools stores (AI account tokens and API keys): an allowed tool can read all of them. Touch ID confirms an allow.") + return lines.joined(separator: "\n") + } + + static func choices(rememberable: Bool) -> [Int?] { + rememberable ? [0, 8 * 3_600, 7 * 86_400, nil] : [0] + } + + static func label(_ seconds: Int?) -> String { + guard let seconds else { return "Always allow this tool" } + if seconds == 0 { return "Allow once" } + if seconds % 86_400 == 0 { return "Allow for \(seconds / 86_400) days" } + return "Allow for \(seconds / 3_600) hours" + } +} + +private func vaultAudit(_ event: String, params: VaultUnlockParams, caller: VaultCaller, session: String?, detail: String = "") { + var parts = ["purpose: \(params.purpose)"] + if let secret = params.secret { parts.append("secret: \(secret)") } + if let session { parts.append("session: \(session)") } + if !detail.isEmpty { parts.append(detail) } + + let entry: [String: Any] = [ + "at": ISO8601DateFormatter().string(from: Date()), + "event": event, + "client": ["name": params.tool, "pid": Int(caller.pid), "executable": caller.executable ?? NSNull(), "cwd": caller.cwd ?? NSNull()], + "account": "vault", + "detail": parts.joined(separator: "; "), + ] + guard let data = try? JSONSerialization.data(withJSONObject: entry, options: [.sortedKeys]) else { return } + let path = (VaultGrants.directory as NSString).appendingPathComponent("audit.jsonl") + try? FileManager.default.createDirectory(atPath: VaultGrants.directory, withIntermediateDirectories: true) + if !FileManager.default.fileExists(atPath: path) { + FileManager.default.createFile(atPath: path, contents: nil, attributes: [.posixPermissions: 0o600]) + } + + guard let handle = FileHandle(forWritingAtPath: path) else { return } + defer { try? handle.close() } + _ = try? handle.seekToEnd() + try? handle.write(contentsOf: data + Data("\n".utf8)) +} + +/// The keychain item the bun rung writes (`genesis-tools` / `master-key`, base64), read as this app. +private func emitVaultKey(remembered: Bool) -> Never { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: "genesis-tools", + kSecAttrAccount as String: "master-key", + kSecReturnData as String: true, + kSecMatchLimit as String: kSecMatchLimitOne, + ] + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + + guard status == errSecSuccess, let data = item as? Data, let key = String(data: data, encoding: .utf8) else { + if status == errSecItemNotFound { + emitError(code: "no_key", message: "the keychain holds no genesis-tools master key", exitCode: 66) + } + + let reason = SecCopyErrorMessageString(status, nil) as String? ?? "status \(status)" + emitError(code: "keychain", message: "GenesisTools could not read the vault key: \(reason)", exitCode: 74) + } + + emitResult(["decision": "allow", "key": key, "remembered": remembered]) +} + +private func vaultDeny(_ reason: String) -> Never { + logClick("vault.unlock decision=deny reason=\(reason)") + emitResult(["decision": "deny", "reason": reason]) +} + +func unlockVault(_ params: VaultUnlockParams) { + let caller = VaultProcess.caller(getppid()) + let session = VaultSession.describe(caller) + guard let executable = caller.executable else { + vaultAudit("denied", params: params, caller: caller, session: session, detail: "the caller's executable could not be read") + vaultDeny("the caller's executable could not be read") + } + + let rememberable = caller.injected.isEmpty + let key = VaultGrants.key(tool: params.tool, executable: executable, script: caller.script) + let now = Int64(Date().timeIntervalSince1970 * 1_000) + + if rememberable, VaultGrants.live(key, now: now, in: VaultGrants.load()) != nil { + vaultAudit("remembered", params: params, caller: caller, session: session) + emitVaultKey(remembered: true) + } + + guard VaultWindowLock.acquire(VaultWindowLock.path(for: key)) != nil else { + vaultAudit("denied", params: params, caller: caller, session: session, detail: "an approval window for this tool is already open") + vaultDeny("an approval window for “\(params.tool)” is already open; answer that one first") + } + + DispatchQueue.main.asyncAfter(deadline: .now() + 170) { + vaultAudit("denied", params: params, caller: caller, session: session, detail: "no answer within 170 s") + vaultDeny("no answer within 170 s") + } + + DispatchQueue.main.async { + NSApplication.shared.activate(ignoringOtherApps: true) + vaultAudit("prompted", params: params, caller: caller, session: session) + + let choices = VaultWindow.choices(rememberable: rememberable) + let alert = NSAlert() + alert.alertStyle = .warning + alert.messageText = VaultWindow.title(params) + alert.informativeText = VaultWindow.body(params, caller: caller, session: session) + for seconds in choices { + alert.addButton(withTitle: VaultWindow.label(seconds)) + } + + let deny = alert.addButton(withTitle: "Deny") + deny.keyEquivalent = "\u{1b}" + + let index = alert.runModal().rawValue - NSApplication.ModalResponse.alertFirstButtonReturn.rawValue + guard index >= 0, index < choices.count else { + vaultAudit("denied", params: params, caller: caller, session: session, detail: "denied in the window") + vaultDeny("denied in the approval window") + } + + let remember = choices[index] + authenticate(reason: "give “\(params.tool)” the GenesisTools vault key") { method, failure in + guard let method else { + vaultAudit("denied", params: params, caller: caller, session: session, detail: failure) + vaultDeny(failure) + } + + if remember != 0 { + let until = remember.map { now + Int64($0) * 1_000 } + let grant = VaultGrant(key: key, tool: params.tool, executable: executable, script: caller.script, + grantedAt: now, until: until, method: method) + VaultGrants.save(VaultGrants.remember(grant, now: now, in: VaultGrants.load())) + } + + vaultAudit("allowed", params: params, caller: caller, session: session, + detail: "method: \(method); remembered: \(remember.map { $0 == 0 ? "once" : "\($0 / 3_600) h" } ?? "always")") + logClick("vault.unlock decision=allow tool=\(params.tool) method=\(method)") + emitVaultKey(remembered: false) + } + } +} diff --git a/src/macos/GenesisTools/Tests/VaultUnlockTests.swift b/src/macos/GenesisTools/Tests/VaultUnlockTests.swift new file mode 100644 index 0000000000..7dbb5af2e5 --- /dev/null +++ b/src/macos/GenesisTools/Tests/VaultUnlockTests.swift @@ -0,0 +1,100 @@ +import XCTest +@testable import GenesisTools + +/// The vault approval window's pure parts (Sources/VaultUnlock.swift): reading a process from the kernel's bytes, +/// what counts as the same tool for a remembered approval, and what the window tells the user. +final class VaultUnlockTests: XCTestCase { + private func procArgsBytes(argc: Int32, executable: String, arguments: [String], environment: [String]) -> [UInt8] { + var bytes = withUnsafeBytes(of: argc.littleEndian) { Array($0) } + bytes += Array(executable.utf8) + [0, 0, 0, 0] + for argument in arguments { + bytes += Array(argument.utf8) + [0] + } + + for entry in environment { + bytes += Array(entry.utf8) + [0] + } + + // The environment ends at an empty entry; the "apple" strings after it are not environment. + bytes += [0] + Array("executable_path=/x".utf8) + [0] + return bytes + } + + private func caller(arguments: [String], environment: [String] = [], ancestors: [(pid_t, String)] = []) -> VaultCaller { + VaultCaller(pid: 4242, executable: "/Users/alice/.genesis-tools/bin/gt-claude", + args: ProcArgs(executable: "/Users/alice/.genesis-tools/bin/gt-claude", arguments: arguments, environment: environment), + cwd: "/Users/alice/project", ancestors: ancestors.map { (pid: $0.0, command: $0.1) }) + } + + func testTheKernelBytesSplitIntoExecutableArgumentsAndEnvironment() throws { + let bytes = procArgsBytes(argc: 3, executable: "/Users/alice/.genesis-tools/bin/gt-claude", + arguments: ["gt-claude", "/repo/src/claude/index.ts", "info"], + environment: ["CLAUDE_CODE_SESSION_ID=0f3a", "HOME=/Users/alice"]) + let parsed = try XCTUnwrap(VaultProcess.parseProcArgs(bytes)) + + XCTAssertEqual(parsed.executable, "/Users/alice/.genesis-tools/bin/gt-claude") + XCTAssertEqual(parsed.arguments, ["gt-claude", "/repo/src/claude/index.ts", "info"]) + XCTAssertEqual(parsed.environment, ["CLAUDE_CODE_SESSION_ID=0f3a", "HOME=/Users/alice"]) + XCTAssertEqual(parsed.environmentValue("CLAUDE_CODE_SESSION_ID"), "0f3a") + } + + func testTheScriptIsPartOfTheIdentityAndLoadersBlockRemembering() { + XCTAssertEqual(caller(arguments: ["gt-claude", "/repo/src/claude/index.ts", "info"]).script, "/repo/src/claude/index.ts") + XCTAssertEqual(caller(arguments: ["gt-claude", "/repo/src/claude/index.ts"]).injected, []) + XCTAssertFalse(caller(arguments: ["gt-claude", "--preload", "/tmp/x.ts", "/repo/src/claude/index.ts"]).injected.isEmpty) + XCTAssertFalse(caller(arguments: ["gt-claude", "/repo/src/claude/index.ts"], environment: ["DYLD_INSERT_LIBRARIES=/tmp/x.dylib"]).injected.isEmpty) + XCTAssertEqual(VaultWindow.choices(rememberable: false), [0], "with extra code loaded, only allow once") + } + + func testARememberedApprovalCoversOnlyTheSameToolExecutableAndScriptUntilItEnds() { + let key = VaultGrants.key(tool: "claude", executable: "/bin/gt-claude", script: "/repo/src/claude/index.ts") + let grant = VaultGrant(key: key, tool: "claude", executable: "/bin/gt-claude", script: "/repo/src/claude/index.ts", + grantedAt: 1_000, until: 2_000, method: "touch-id") + let file = VaultGrants.remember(grant, now: 1_000, in: VaultGrantsFile(version: 1, grants: [])) + + XCTAssertNotNil(VaultGrants.live(key, now: 1_500, in: file)) + XCTAssertNil(VaultGrants.live(key, now: 2_000, in: file), "a timed approval ends") + XCTAssertNil(VaultGrants.live(VaultGrants.key(tool: "claude", executable: "/bin/gt-claude", script: "/tmp/other.ts"), now: 1_500, in: file)) + XCTAssertNil(VaultGrants.live(VaultGrants.key(tool: "say", executable: "/bin/gt-claude", script: "/repo/src/claude/index.ts"), now: 1_500, in: file)) + + var always = grant + always.until = nil + XCTAssertNotNil(VaultGrants.live(key, now: Int64.max, in: VaultGrants.remember(always, now: 1_000, in: file)), "always lasts until revoked") + } + + func testTheWindowSaysWhyWhatWhoAndWhichSession() { + let params = VaultUnlockParams(tool: "claude", purpose: "match the session's token to an account for the statusline", + secret: "ai/acc_work/longLivedToken", command: "tools claude info") + let who = caller(arguments: ["gt-claude", "/repo/src/claude/index.ts", "info"], + environment: ["CLAUDE_CODE_SESSION_ID=0f3a-session", "CMUX_SURFACE_ID=surface-9"], + ancestors: [(100, "GenesisTools gt-claude"), (90, "zsh"), (80, "claude")]) + let session = VaultSession.describe(who) + let body = VaultWindow.body(params, caller: who, session: session) + + XCTAssertEqual(session, "Claude Code session 0f3a-session, cmux tab surface-9") + XCTAssertTrue(body.contains("Why: match the session's token to an account for the statusline")) + XCTAssertTrue(body.contains("It was reading: ai/acc_work/longLivedToken")) + XCTAssertTrue(body.contains("Executable: /Users/alice/.genesis-tools/bin/gt-claude")) + XCTAssertTrue(body.contains("Script: /repo/src/claude/index.ts")) + XCTAssertTrue(body.contains("Started by: GenesisTools gt-claude (100) ← zsh (90) ← claude (80)")) + XCTAssertTrue(body.contains("Session: Claude Code session 0f3a-session")) + XCTAssertFalse(body.contains("Extra code")) + } + + func testASecondRequestFromTheSameToolGetsNoSecondWindow() throws { + let path = (NSTemporaryDirectory() as NSString).appendingPathComponent("vault-window-test-\(UUID().uuidString).lock") + defer { try? FileManager.default.removeItem(atPath: path) } + let first = try XCTUnwrap(VaultWindowLock.acquire(path)) + + XCTAssertNil(VaultWindowLock.acquire(path), "the first window holds the lock") + close(first) + XCTAssertNotNil(VaultWindowLock.acquire(path), "a closed window frees it") + XCTAssertNotEqual(VaultWindowLock.path(for: "vault|claude|/a|"), VaultWindowLock.path(for: "vault|say|/a|")) + } + + func testWithoutASessionIdTheAgentInTheParentChainNamesIt() { + let who = caller(arguments: ["gt-say", "/repo/src/say/index.ts"], ancestors: [(90, "zsh"), (80, "codex exec")]) + + XCTAssertEqual(VaultSession.describe(who), "codex exec (pid 80)") + } +} diff --git a/src/say/lib/credential.ts b/src/say/lib/credential.ts index b9b21ad22a..7e169067fa 100644 --- a/src/say/lib/credential.ts +++ b/src/say/lib/credential.ts @@ -4,6 +4,7 @@ import { resolveProviderApiKey } from "@genesiscz/utils/ai/providers/resolve"; import { toolCommand } from "@genesiscz/utils/cli/tool-command"; import { logger } from "@genesiscz/utils/logger"; import type { SayProvider } from "@genesiscz/utils/macos/SayConfigManager"; +import { withVaultPurpose } from "@genesiscz/utils/security/vault-purpose"; export type SayCredentialCheck = | { kind: "ok"; apiKey?: string } @@ -173,9 +174,9 @@ export async function sayAccountKey(account: SayAccount, deps: SayAccountDeps = logger.debug({ account: account.entry.name, provider: account.provider, gated }, "[say] resolving the account key"); try { - return gated - ? await (deps.requestGate ?? gateKey)(account.entry) - : await (deps.resolveKey ?? accountKey)(account.entry); + return await withVaultPurpose(`speak through ${account.provider} with the account ${account.entry.name}`, () => + gated ? (deps.requestGate ?? gateKey)(account.entry) : (deps.resolveKey ?? accountKey)(account.entry) + ); } catch (err) { if (err instanceof SayAccountError) { throw err; diff --git a/src/utils/ai/gate/grants.ts b/src/utils/ai/gate/grants.ts index c86bb3ac4d..df2cb67f38 100644 --- a/src/utils/ai/gate/grants.ts +++ b/src/utils/ai/gate/grants.ts @@ -8,7 +8,7 @@ import { logger } from "@genesiscz/utils/logger"; import { isJsonObject, readJsonFileForUpdateSync } from "@genesiscz/utils/storage/atomic-file"; import { withFileLock } from "@genesiscz/utils/storage/file-lock"; import { atomicWriteFileSync } from "@genesiscz/utils/storage/storage"; -import type { ClientIdentity, GateGrant, GateProvider, TokenKind } from "./types"; +import type { ClientIdentity, GateGrant, GateProvider, TokenKind, VaultGrant } from "./types"; const { log } = logger.scoped("ai-gate"); @@ -123,6 +123,56 @@ export async function rememberGrant(grant: GateGrant): Promise { ]); } +export function vaultGrantsPath(): string { + return join(gateDir(), "vault-grants.json"); +} + +function isVaultGrant(value: unknown): value is VaultGrant { + return isJsonObject(value) && typeof value.key === "string" && typeof value.tool === "string"; +} + +function readVaultGrants(): VaultGrant[] { + const path = vaultGrantsPath(); + if (!existsSync(path)) { + return []; + } + + const parsed = parseJSON<{ grants?: unknown[] }>(readFileSync(path, "utf8")); + return (parsed?.grants ?? []).filter(isVaultGrant); +} + +/** Vault-key approvals GenesisTools.app remembers that have not ended. */ +export function listVaultGrants(now = Date.now()): VaultGrant[] { + return readVaultGrants().filter((grant) => grant.until === null || grant.until > now); +} + +/** Drop vault-key approvals by tool name or key; `"*"` drops every one. The app writes this file by atomic rename. */ +export async function revokeVaultGrants(selector: string): Promise { + const path = vaultGrantsPath(); + let removed = 0; + + if (!existsSync(path)) { + return 0; + } + + await withFileLock( + `${path}.lock`, + async () => { + const grants = readVaultGrants(); + const kept = grants.filter( + (grant) => selector !== "*" && grant.key !== selector && grant.tool !== selector + ); + removed = grants.length - kept.length; + atomicWriteFileSync(path, `${SafeJSON.stringify({ version: 1, grants: kept }, null, 2)}\n`, { + mode: 0o600, + }); + }, + 10_000 + ); + + return removed; +} + /** Drop grants by client key or client name; `"*"` drops every grant. Returns how many went. */ export async function revokeGrants(selector: string): Promise { let removed = 0; diff --git a/src/utils/ai/gate/index.ts b/src/utils/ai/gate/index.ts index cbe0906a62..da812e873a 100644 --- a/src/utils/ai/gate/index.ts +++ b/src/utils/ai/gate/index.ts @@ -13,9 +13,12 @@ export { gateDir, grantsPath, listGrants, + listVaultGrants, readAuditTail, rememberGrant, revokeGrants, + revokeVaultGrants, + vaultGrantsPath, } from "./grants"; export { type GateDeps, providerTokenResolver, requestAccountAccess, type TokenResolver } from "./request"; export * from "./types"; diff --git a/src/utils/ai/gate/types.ts b/src/utils/ai/gate/types.ts index 8a8fd84826..92d153222d 100644 --- a/src/utils/ai/gate/types.ts +++ b/src/utils/ai/gate/types.ts @@ -91,6 +91,21 @@ export type ApprovalDecision = | { decision: "deny"; reason: string }; /** A remembered approval. Matched on client key, provider, account id and the token kind approved. */ +/** + * A remembered vault-key approval, written by GenesisTools.app's vault window (src/macos/GenesisTools/Sources/ + * VaultUnlock.swift) into `vault-grants.json`. One tool, one executable, one script; `until` null means "always", + * until `tools ai gate revoke` drops it. + */ +export interface VaultGrant { + key: string; + tool: string; + executable: string; + script?: string | null; + grantedAt: number; + until: number | null; + method: string; +} + export interface GateGrant { key: string; clientName: string; diff --git a/src/utils/macos/genesis-app-rpc.ts b/src/utils/macos/genesis-app-rpc.ts index fa164fceb0..2974d16a3a 100644 --- a/src/utils/macos/genesis-app-rpc.ts +++ b/src/utils/macos/genesis-app-rpc.ts @@ -115,6 +115,56 @@ export async function genesisAppRpc( } /** Exported for tests: this is where a reply from something other than the app has to be caught. */ +/** + * `genesisAppRpc` for a caller that cannot await: the synchronous vault-key read behind `resolveSecretSync`. It blocks + * the event loop until the app answers, which for an approval window is up to `timeoutMs`; use it only where a sync + * signature leaves no choice. + */ +export function genesisAppRpcSync( + method: string, + params?: Record, + opts: GenesisAppRpcOptions = {} +): GenesisAppRpcOutcome { + const launcher = installedGenesisAppLauncher(); + + if (!launcher) { + return { + ok: false, + error: { code: "unavailable", message: "GenesisTools.app is not installed, or routing is switched off" }, + }; + } + + const request = SafeJSON.stringify(params ? { method, params: stripUndefined(params) } : { method }); + const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const proc = Bun.spawnSync([launcher, "--rpc", request], { + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + timeout: timeoutMs, + }); + const stderr = proc.stderr.toString(); + + if (proc.exitedDueToTimeout) { + logger.debug({ method, timeoutMs }, "GenesisTools.app RPC timed out"); + return { ok: false, error: { code: "timeout", message: `${method} timed out after ${timeoutMs}ms` } }; + } + + const line = proc.stdout.toString().trim().split("\n").filter(Boolean).at(-1); + + if (!line) { + logger.debug({ method, exitCode: proc.exitCode, stderr }, "GenesisTools.app RPC returned no reply line"); + return { + ok: false, + error: { + code: "handshake", + message: `${method} produced no reply (exit ${proc.exitCode})${stderr.trim() ? `: ${stderr.trim()}` : ""}`, + }, + }; + } + + return parseReply(method, line, proc.exitCode ?? -1, stderr, opts.isResult); +} + export function parseReply( method: string, line: string, diff --git a/src/utils/security/MasterKey.ts b/src/utils/security/MasterKey.ts index 21e294b024..e7aaed70d5 100644 --- a/src/utils/security/MasterKey.ts +++ b/src/utils/security/MasterKey.ts @@ -6,6 +6,7 @@ import { toolCommand } from "@genesiscz/utils/cli/tool-command"; import { env } from "@genesiscz/utils/env"; import { SafeJSON } from "@genesiscz/utils/json"; import { logger } from "@genesiscz/utils/logger"; +import { appGateKey } from "./keyring/app-gate"; import { envKeyProvider, fileKeyProvider, masterKeyFilePath, securityStorage } from "./keyring/headless"; import { osKeyring } from "./keyring/os-keyring"; import { MASTER_KEY_BYTES, type MasterKeyProvider, type MasterKeySource } from "./keyring/types"; @@ -30,10 +31,13 @@ interface KeyState { } let cached: KeyState | null = null; -let providers: MasterKeyProvider[] = [envKeyProvider, osKeyring, fileKeyProvider]; +// The app rung sits before the keychain: with GenesisTools.app installed, the app's approval window replaces the +// keychain's per-executable dialog. It answers nothing when the app cannot (then the keychain does), and throws on a +// deny, so a refused key never falls through to the dialog it replaces. +let providers: MasterKeyProvider[] = [envKeyProvider, appGateKey, osKeyring, fileKeyProvider]; /** - * Ladder, first hit wins: in-process cache, env, OS keychain, opt-in key file. + * Ladder, first hit wins: in-process cache, env, GenesisTools.app's approval window, OS keychain, opt-in key file. * Interactive processes generate and store a key when every rung misses; * headless ones fail loudly rather than minting a key nobody can find later. * @@ -238,7 +242,8 @@ export async function writeMasterKey(key: Buffer): Promise { } for (const provider of providers) { - if (provider.id === "env") { + // The env rung is the caller's own override and the app rung only reads, so neither stores a new key. + if (provider.id === "env" || provider.id === "app") { continue; } diff --git a/src/utils/security/SecretStore.ts b/src/utils/security/SecretStore.ts index f7ed06b57b..0504d6813b 100644 --- a/src/utils/security/SecretStore.ts +++ b/src/utils/security/SecretStore.ts @@ -15,6 +15,7 @@ import { } from "./MasterKey"; import { isSecretPath, isSecureRef, type MaybeSecret, type SecureRef, secureRef } from "./SecureRef"; import { emptyVault, VAULT_HKDF_SALT, VAULT_VERSION, type VaultEntry, type VaultFile } from "./vault-format"; +import { noteVaultSecret } from "./vault-purpose"; const KEY_BYTES = 32; const IV_BYTES = 12; @@ -192,6 +193,7 @@ class FileSecretStore implements SecretStore { return undefined; } + noteVaultSecret(path); try { return decryptEntry(await masterKeyForId(vault.keyId), path, entry); } catch (err) { @@ -214,6 +216,7 @@ class FileSecretStore implements SecretStore { return undefined; } + noteVaultSecret(path); key ??= masterKeyForId(vault.keyId); try { return decryptEntry(await key, path, entry); @@ -240,6 +243,7 @@ class FileSecretStore implements SecretStore { return undefined; } + noteVaultSecret(path); const key = masterKeyForIdSync(snapshot.keyId); if (!key) { projectionMissedKey = true; diff --git a/src/utils/security/keyring/app-gate.test.ts b/src/utils/security/keyring/app-gate.test.ts new file mode 100644 index 0000000000..3680ef6a0a --- /dev/null +++ b/src/utils/security/keyring/app-gate.test.ts @@ -0,0 +1,148 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { _setMasterKeyProvidersForTest, masterKey, masterKeySync, writeMasterKey } from "../MasterKey"; +import { + _resetVaultContextForTest, + currentCommandLine, + currentToolName, + noteVaultSecret, + setVaultPurpose, +} from "../vault-purpose"; +import { appGateKey, createAppGateKeyProvider, VaultKeyDeniedError, type VaultUnlockRequest } from "./app-gate"; +import type { MasterKeyProvider } from "./types"; + +const KEY = Buffer.alloc(32, 7); +const allow = { ok: true as const, result: { decision: "allow" as const, key: KEY.toString("base64") } }; +const deny = { ok: true as const, result: { decision: "deny" as const, reason: "denied in the approval window" } }; +const failure = (code: string) => ({ ok: false as const, error: { code, message: `${code} from the fake app` } }); + +/** A keychain stand-in that records reads and writes; `throwOnRead` proves a path never reached it. */ +function fakeKeychain(stored: Buffer | undefined, throwOnRead = false) { + const calls = { reads: 0, writes: [] as Buffer[] }; + const rung: MasterKeyProvider = { + id: "keychain", + available: async () => true, + get: async () => rung.getSync?.(), + getSync: () => { + calls.reads++; + if (throwOnRead) { + throw new Error("the keychain was read after the app denied the key"); + } + + return stored; + }, + set: async (key) => { + calls.writes.push(key); + }, + }; + return { rung, calls }; +} + +afterEach(() => { + _resetVaultContextForTest(); + _setMasterKeyProvidersForTest([]); +}); + +describe("the app rung", () => { + test("an allow returns the key the app read", async () => { + const rung = createAppGateKeyProvider({ available: () => true, rpc: async () => allow, rpcSync: () => allow }); + + expect(await rung.get()).toEqual(KEY); + expect(rung.getSync?.()).toEqual(KEY); + }); + + test("a deny throws with the window's reason and never reaches the keychain", () => { + const keychain = fakeKeychain(KEY, true); + _setMasterKeyProvidersForTest([ + createAppGateKeyProvider({ available: () => true, rpcSync: () => deny }), + keychain.rung, + ]); + + expect(() => masterKeySync()).toThrow(VaultKeyDeniedError); + expect(() => masterKeySync()).toThrow("denied in the approval window"); + expect(keychain.calls.reads).toBe(0); + }); + + test("an app that cannot answer lets the keychain answer, as before the app existed", async () => { + for (const code of ["unavailable", "method_unknown", "handshake", "no_key"]) { + const keychain = fakeKeychain(KEY); + const app = createAppGateKeyProvider({ + available: () => true, + rpc: async () => failure(code), + rpcSync: () => failure(code), + }); + _setMasterKeyProvidersForTest([app, keychain.rung]); + + expect(masterKeySync()).toEqual(KEY); + expect(keychain.calls.reads).toBe(1); + } + }); + + test("no answer within the window's time is a deny, not a fall-through to the keychain dialog", () => { + const app = createAppGateKeyProvider({ available: () => true, rpcSync: () => failure("timeout") }); + + expect(() => app.getSync?.()).toThrow("timeout"); + }); + + test("without the app the rung asks nothing and the keychain answers", async () => { + let asked = 0; + const keychain = fakeKeychain(KEY); + const app = createAppGateKeyProvider({ + available: () => false, + rpc: async () => { + asked++; + return allow; + }, + }); + _setMasterKeyProvidersForTest([app, keychain.rung]); + + expect(await masterKey()).toEqual(KEY); + expect(asked).toBe(0); + }); + + test("the request carries why, what and the command", async () => { + const seen: VaultUnlockRequest[] = []; + setVaultPurpose("refresh the usage figures of 3 accounts for the statusline"); + noteVaultSecret("ai/acc_work/accessToken"); + noteVaultSecret("ai/acc_side/accessToken"); + const app = createAppGateKeyProvider({ + available: () => true, + rpc: async (request) => { + seen.push(request); + return allow; + }, + }); + + await app.get(); + expect(seen[0]?.purpose).toBe("refresh the usage figures of 3 accounts for the statusline"); + expect(seen[0]?.secret).toBe("ai/acc_work/accessToken"); + expect(seen[0]?.command.startsWith("tools ")).toBe(true); + }); + + test("a new key is stored by the keychain, never by the app rung", async () => { + const keychain = fakeKeychain(undefined); + _setMasterKeyProvidersForTest([ + createAppGateKeyProvider({ available: () => true, rpc: async () => allow }), + keychain.rung, + ]); + + expect(await writeMasterKey(KEY)).toBe("keychain"); + expect(keychain.calls.writes).toEqual([KEY]); + }); + + test("a test process never reaches the real app", async () => { + expect(await appGateKey.available()).toBe(false); + expect(appGateKey.getSync?.()).toBeUndefined(); + }); +}); + +describe("the caller's name and command", () => { + test("from the tool's entry script, else from its gt- launcher", () => { + expect(currentToolName(["/Users/x/.genesis-tools/bin/gt-claude", "/repo/src/claude/index.ts", "info"])).toBe( + "claude" + ); + expect(currentToolName(["/Users/x/.genesis-tools/bin/gt-say", "/somewhere/else.ts"])).toBe("say"); + expect(currentCommandLine(["gt-say", "/repo/src/say/index.ts", "hello there", "--app", "claude"])).toBe( + 'tools say "hello there" --app claude' + ); + }); +}); diff --git a/src/utils/security/keyring/app-gate.ts b/src/utils/security/keyring/app-gate.ts new file mode 100644 index 0000000000..c9f1ac8ef5 --- /dev/null +++ b/src/utils/security/keyring/app-gate.ts @@ -0,0 +1,148 @@ +import { logger } from "@genesiscz/utils/logger"; +import { + type GenesisAppRpcOutcome, + genesisAppRpc, + genesisAppRpcSync, + isGenesisAppRpcAvailable, +} from "@genesiscz/utils/macos/genesis-app-rpc"; +import { currentCommandLine, currentToolName, vaultPurpose, vaultSecretAsked } from "../vault-purpose"; +import { decodeMasterKey, isTestProcess, type MasterKeyProvider } from "./types"; + +/** + * The vault master key through GenesisTools.app's approval window (Martin's DECISION 131 b, 2026-10-15). + * + * Every tool runs as its own `gt-` link of bun, and macOS keys a keychain item's access list on the executable + * path, so each tool used to raise its own "gt-claude wants to use your confidential information" dialog with nothing + * in it to judge. Here the app reads the item as itself (one macOS prompt, ever, naming GenesisTools) and hands the key + * on only after its own window: WHO from the kernel (the app is this process's child), WHICH SESSION from this + * process's environment, WHY and WHAT from the request below. A remembered approval skips the window. + * + * A deny throws: falling through to the keychain rung would bring back the dialog this rung exists to replace. An app + * that cannot answer (not installed, switched off, an older build without `vault.unlock`) returns undefined, and the + * keychain rung answers as before. + */ + +/** A person reads the window and touches the sensor; the app gives up at 170 s and says so. */ +export const VAULT_UNLOCK_TIMEOUT_MS = 180_000; + +export interface VaultUnlockRequest { + /** The tool as GenesisTools names it (`claude`, `say`); the app shows it beside the executable it verified. */ + tool: string; + /** Why, in one sentence: the caller's own, else the command line. */ + purpose: string; + /** The secret this process was reading when it needed the key (`ai//`). */ + secret?: string; + command: string; +} + +export interface VaultUnlockReply { + decision: "allow" | "deny"; + /** The keychain item as stored: base64 of the 32-byte key. Present only with "allow". */ + key?: string; + reason?: string; + /** True when a remembered approval answered and no window opened. */ + remembered?: boolean; +} + +export class VaultKeyDeniedError extends Error { + constructor(tool: string, reason: string) { + super( + `GenesisTools.app did not give the vault key to “${tool}”: ${reason}. ` + + "Allow it in the approval window, or set GENESIS_TOOLS_NO_APP=1 to read the keychain directly." + ); + this.name = "VaultKeyDeniedError"; + } +} + +/** App errors that mean "nobody here can decide": the keychain rung answers instead. */ +const FALL_THROUGH = new Set(["unavailable", "method_unknown", "handshake", "no_key"]); + +export function vaultUnlockRequest(): VaultUnlockRequest { + const command = currentCommandLine(); + + return { + tool: currentToolName(), + purpose: vaultPurpose() ?? `run ${command}`, + secret: vaultSecretAsked(), + command, + }; +} + +function isReply(value: unknown): value is VaultUnlockReply { + return typeof value === "object" && value !== null && "decision" in value; +} + +/** The app's answer as a key, undefined (let the next rung answer), or a thrown deny. */ +export function keyFromReply(outcome: GenesisAppRpcOutcome, tool: string): Buffer | undefined { + if (!outcome.ok) { + if (FALL_THROUGH.has(outcome.error.code)) { + logger.debug( + { code: outcome.error.code, message: outcome.error.message }, + "vault key: the app cannot answer" + ); + return undefined; + } + + throw new VaultKeyDeniedError(tool, `${outcome.error.code}: ${outcome.error.message}`); + } + + if (outcome.result.decision !== "allow") { + throw new VaultKeyDeniedError(tool, outcome.result.reason ?? "denied in the approval window"); + } + + const key = decodeMasterKey(outcome.result.key, "app"); + if (!key) { + throw new VaultKeyDeniedError(tool, "the app allowed it but returned no readable key"); + } + + logger.debug({ tool, remembered: outcome.result.remembered === true }, "vault key handed down by GenesisTools.app"); + return key; +} + +export interface AppGateDeps { + available?: () => boolean; + rpc?: (request: VaultUnlockRequest) => Promise>; + rpcSync?: (request: VaultUnlockRequest) => GenesisAppRpcOutcome; +} + +/** Tests build the rung with fakes; production uses `appGateKey`. A test process never reaches the real app. */ +export function createAppGateKeyProvider(deps: AppGateDeps = {}): MasterKeyProvider { + const available = deps.available ?? (() => !isTestProcess() && isGenesisAppRpcAvailable()); + const options = { timeoutMs: VAULT_UNLOCK_TIMEOUT_MS, isResult: isReply }; + const params = (request: VaultUnlockRequest): Record => ({ ...request }); + const rpc = + deps.rpc ?? + ((request: VaultUnlockRequest) => genesisAppRpc("vault.unlock", params(request), options)); + const rpcSync = + deps.rpcSync ?? + ((request: VaultUnlockRequest) => + genesisAppRpcSync("vault.unlock", params(request), options)); + + return { + id: "app", + async available() { + return available(); + }, + async get() { + if (!available()) { + return undefined; + } + + const request = vaultUnlockRequest(); + return keyFromReply(await rpc(request), request.tool); + }, + getSync() { + if (!available()) { + return undefined; + } + + const request = vaultUnlockRequest(); + return keyFromReply(rpcSync(request), request.tool); + }, + async set() { + throw new Error("GenesisTools.app only reads the vault key; a new key is stored by the keychain rung"); + }, + }; +} + +export const appGateKey: MasterKeyProvider = createAppGateKeyProvider(); diff --git a/src/utils/security/keyring/types.ts b/src/utils/security/keyring/types.ts index 204cc3c651..3045f9dead 100644 --- a/src/utils/security/keyring/types.ts +++ b/src/utils/security/keyring/types.ts @@ -1,6 +1,6 @@ import { isTestProcess } from "@genesiscz/utils/test-process"; -export type MasterKeySource = "keychain" | "env" | "file"; +export type MasterKeySource = "keychain" | "env" | "file" | "app"; export interface MasterKeyProvider { readonly id: MasterKeySource; diff --git a/src/utils/security/vault-purpose.ts b/src/utils/security/vault-purpose.ts new file mode 100644 index 0000000000..585deef703 --- /dev/null +++ b/src/utils/security/vault-purpose.ts @@ -0,0 +1,69 @@ +import { basename } from "node:path"; + +/** + * Why this process reads the vault, for the approval window GenesisTools.app shows before it hands the master key to a + * tool. Martin, 2026-10-15: "enough context so i know exactly what / who / which session / triggered that and why + * (each caller should give that context possibly)". WHO and WHICH SESSION the app reads from the kernel; WHY and WHAT + * only the caller knows, so they live here. + */ +let purpose: string | undefined; +let firstSecret: string | undefined; + +/** One sentence a person can judge: "refresh the usage figures of 11 accounts for the statusline". */ +export function setVaultPurpose(text: string | undefined): void { + purpose = text?.trim() || undefined; +} + +/** The purpose for the duration of `fn`, then the one before it again. */ +export async function withVaultPurpose(text: string, fn: () => Promise): Promise { + const previous = purpose; + purpose = text.trim() || previous; + + try { + return await fn(); + } finally { + purpose = previous; + } +} + +export function vaultPurpose(): string | undefined { + return purpose; +} + +/** The first secret this process read: what it was after when it needed the master key. */ +export function noteVaultSecret(path: string): void { + firstSecret ??= path; +} + +export function vaultSecretAsked(): string | undefined { + return firstSecret; +} + +/** Test seam. */ +export function _resetVaultContextForTest(): void { + purpose = undefined; + firstSecret = undefined; +} + +/** The tool this process runs: `src//index.ts` from the script path, else the `gt-` launcher name. */ +export function currentToolName(argv: string[] = process.argv): string { + const script = argv[1] ?? ""; + const fromScript = script.match(/[/\\]src[/\\]([^/\\]+)[/\\]index\.tsx?$/)?.[1]; + + if (fromScript) { + return fromScript; + } + + const launcher = basename(argv[0] ?? ""); + return launcher.startsWith("gt-") ? launcher.slice(3) : launcher || "unknown"; +} + +/** `tools `, cut to a length a window can show. Tokens never travel in argv (they go through stdin). */ +export function currentCommandLine(argv: string[] = process.argv, max = 300): string { + const words = ["tools", currentToolName(argv), ...argv.slice(2)].map((word) => + word.includes(" ") ? `"${word}"` : word + ); + const line = words.join(" "); + + return line.length > max ? `${line.slice(0, max - 1)}…` : line; +} From 1da3a69bb2fff65b2e6dfb5fbb55ee41d986b4fa Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 15 Oct 2026 00:43:56 +0200 Subject: [PATCH 14/32] fix(ai, macos): the vault and account approval windows are one SwiftUI panel that leads with why and folds the details, time out for real, remember a program by its script and ask the keychain password once, because the first window was a wall of text, kept reopening and asked for the password twice on every allow (cherry picked from commit d5b318467e522e585599c1d32a8b1ef656240fa8) --- .../Permissions/ApprovalDialog.swift | 297 +++++++++++ src/macos/GenesisTools/Sources/Gate.swift | 200 ++++---- .../GenesisTools/Sources/VaultUnlock.swift | 469 +++++++++++++----- .../GenesisTools/Tests/VaultUnlockTests.swift | 157 ++++-- src/utils/security/keyring/app-gate.test.ts | 28 ++ src/utils/security/keyring/app-gate.ts | 32 +- src/utils/security/vault-purpose.ts | 19 +- 7 files changed, 938 insertions(+), 264 deletions(-) create mode 100644 src/macos/GenesisKit/Sources/GenesisKit/Permissions/ApprovalDialog.swift diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Permissions/ApprovalDialog.swift b/src/macos/GenesisKit/Sources/GenesisKit/Permissions/ApprovalDialog.swift new file mode 100644 index 0000000000..7857aa43d7 --- /dev/null +++ b/src/macos/GenesisKit/Sources/GenesisKit/Permissions/ApprovalDialog.swift @@ -0,0 +1,297 @@ +import AppKit +import SwiftUI + +/// One line of an approval request: a short label and its value. `help` holds the full text when the value is cut. +public struct ApprovalFact: Identifiable, Equatable { + public let label: String + public let value: String + public let help: String? + public let monospaced: Bool + + public var id: String { label } + + public init(_ label: String, _ value: String, help: String? = nil, monospaced: Bool = false) { + self.label = label + self.value = value + self.help = help + self.monospaced = monospaced + } +} + +/// How long an allow is remembered; `seconds` 0 is "just this once", nil is "always" (until revoked). +public struct ApprovalRemember: Identifiable, Hashable { + public let seconds: Int? + public let title: String + + public var id: String { title } + + public init(seconds: Int?, title: String) { + self.seconds = seconds + self.title = title + } +} + +/// A request only the user may allow (a secret handed to another program): why first, then who, then the details +/// behind a disclosure. The answer arrives once, through `onAllow` or `onDeny`. +@MainActor +public final class ApprovalDialogModel: ObservableObject { + public let title: String + public let purpose: String + public let symbol: String + public let facts: [ApprovalFact] + public let details: [ApprovalFact] + public let warning: String? + public let footnote: String + public let rememberChoices: [ApprovalRemember] + public let rememberNote: String? + public let allowTitle: String + public let deadline: Date? + public let identifier: String + + @Published public var rememberIndex = 0 + @Published public var showsDetails = false + @Published public private(set) var answered = false + + public var onAllow: @MainActor (ApprovalRemember?) -> Void = { _ in } + public var onDeny: @MainActor () -> Void = {} + + public init(title: String, purpose: String, symbol: String, facts: [ApprovalFact], details: [ApprovalFact], + warning: String?, footnote: String, rememberChoices: [ApprovalRemember], rememberNote: String?, + allowTitle: String, deadline: Date?, identifier: String) { + self.title = title + self.purpose = purpose + self.symbol = symbol + self.facts = facts + self.details = details + self.warning = warning + self.footnote = footnote + self.rememberChoices = rememberChoices + self.rememberNote = rememberNote + self.allowTitle = allowTitle + self.deadline = deadline + self.identifier = identifier + } + + public func allow() { + guard !answered else { return } + answered = true + onAllow(rememberChoices.indices.contains(rememberIndex) ? rememberChoices[rememberIndex] : nil) + } + + public func deny() { + guard !answered else { return } + answered = true + onDeny() + } +} + +/// Laid out like a macOS alert: the app icon with the request's symbol, a short title, the reason as the first line a +/// reader sees, a column of facts, the rest folded, and the buttons on the trailing edge with the default last. +public struct ApprovalDialogView: View { + @ObservedObject var model: ApprovalDialogModel + + public init(model: ApprovalDialogModel) { + self.model = model + } + + public var body: some View { + VStack(alignment: .leading, spacing: 14) { + HStack(alignment: .top, spacing: 14) { + icon + VStack(alignment: .leading, spacing: 4) { + Text(model.title) + .font(.system(size: 13, weight: .semibold)) + .accessibilityAddTraits(.isHeader) + Text(model.purpose) + .font(.system(size: 12)) + .fixedSize(horizontal: false, vertical: true) + .textSelection(.enabled) + } + .frame(maxWidth: .infinity, alignment: .leading) + } + + facts(model.facts) + .padding(.leading, 66) + + GenDisclosure(isExpanded: $model.showsDetails, minHeight: 22, identifier: "\(model.identifier)-details") { + facts(model.details) + .padding(.leading, 18) + .padding(.top, 2) + } label: { + Text("Details") + .font(.system(size: 12)) + .foregroundStyle(.secondary) + } + .padding(.leading, 66) + + if let warning = model.warning { + Label(warning, systemImage: "exclamationmark.triangle.fill") + .font(.system(size: 11)) + .foregroundStyle(.orange) + .fixedSize(horizontal: false, vertical: true) + .padding(.leading, 66) + .accessibilityIdentifier("\(model.identifier)-warning") + } + + Text(model.footnote) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + .padding(.leading, 66) + + buttons + } + .padding(20) + .frame(width: 480) + .accessibilityElement(children: .contain) + .accessibilityIdentifier(model.identifier) + } + + private var icon: some View { + ZStack(alignment: .bottomTrailing) { + Image(nsImage: NSApp?.applicationIconImage ?? NSImage()) + .resizable() + .frame(width: 52, height: 52) + Image(systemName: model.symbol) + .font(.system(size: 11, weight: .semibold)) + .foregroundStyle(.white) + .frame(width: 22, height: 22) + .background(Circle().fill(Color.orange)) + .overlay(Circle().stroke(Color(nsColor: .windowBackgroundColor), lineWidth: 2)) + .offset(x: 4, y: 4) + } + .frame(width: 56, height: 56, alignment: .topLeading) + .accessibilityHidden(true) + } + + private func facts(_ rows: [ApprovalFact]) -> some View { + Grid(alignment: .leadingFirstTextBaseline, horizontalSpacing: 10, verticalSpacing: 5) { + ForEach(rows) { fact in + GridRow { + Text(fact.label) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .gridColumnAlignment(.trailing) + Text(verbatim: fact.value) + .font(fact.monospaced ? .system(size: 11, design: .monospaced) : .system(size: 12)) + .lineLimit(3) + .truncationMode(.middle) + .textSelection(.enabled) + .help(fact.help ?? fact.value) + .frame(maxWidth: .infinity, alignment: .leading) + } + } + } + } + + private var buttons: some View { + HStack(spacing: 8) { + Button("Deny", action: model.deny) + .keyboardShortcut(.cancelAction) + .accessibilityIdentifier("\(model.identifier)-deny") + if let deadline = model.deadline { + (Text("Denies itself in ") + Text(timerInterval: Date()...max(Date(), deadline), countsDown: true)) + .font(.system(size: 11).monospacedDigit()) + .foregroundStyle(.tertiary) + } + Spacer(minLength: 8) + if model.rememberChoices.count > 1 { + Picker("Remember", selection: $model.rememberIndex) { + ForEach(model.rememberChoices.indices, id: \.self) { index in + Text(model.rememberChoices[index].title).tag(index) + } + } + .labelsHidden() + .pickerStyle(.menu) + .fixedSize() + .accessibilityIdentifier("\(model.identifier)-remember") + } else if let note = model.rememberNote { + Text(note) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .lineLimit(2) + .frame(maxWidth: 170, alignment: .trailing) + .multilineTextAlignment(.trailing) + } + Button(model.allowTitle, action: model.allow) + .keyboardShortcut(.defaultAction) + .buttonStyle(.borderedProminent) + .accessibilityIdentifier("\(model.identifier)-allow") + } + .controlSize(.regular) + .disabled(model.answered) + // The panel is non-activating, so its app is never active while it shows; AppKit then greys the prominent + // button and the main action looks like the others (the permission dialog's live check, 2026-10-10). + .environment(\.controlActiveState, .key) + } +} + +/// One floating panel for one request. It never runs a modal session (a modal loop on the main queue stops every +/// timer queued there, so a deadline never fired), never activates its app, and takes the keyboard only after the +/// user clicks into it: a request that pops up while someone types must not catch a stray Return. +@MainActor +public final class ApprovalPanelPresenter: NSObject, NSWindowDelegate { + private var panel: NSPanel? + private var model: ApprovalDialogModel? + + public override init() {} + + public func present(_ model: ApprovalDialogModel) { + if NSApp == nil { _ = NSApplication.shared } + let controller = NSHostingController(rootView: ApprovalDialogView(model: model).titlebarZone()) + // The panel follows the view's size, so opening Details grows it and closing shrinks it back. + controller.sizingOptions = [.preferredContentSize] + let size = controller.view.fittingSize + let panel = ApprovalPanel( + contentRect: CGRect(origin: .zero, size: size), + styleMask: [.titled, .closable, .fullSizeContentView, .nonactivatingPanel], + backing: .buffered, defer: false) + panel.titleVisibility = .hidden + panel.titlebarAppearsTransparent = true + for button in [NSWindow.ButtonType.closeButton, .miniaturizeButton, .zoomButton] { + panel.standardWindowButton(button)?.isHidden = true + } + panel.title = model.title + panel.hidesOnDeactivate = false + panel.isReleasedWhenClosed = false + panel.isFloatingPanel = true + // After isFloatingPanel, whose setter resets the level: above the widget's panels, as the permission dialog. + panel.level = NSWindow.Level(rawValue: NSWindow.Level.statusBar.rawValue + 1) + panel.collectionBehavior = [.moveToActiveSpace, .fullScreenAuxiliary] + panel.animationBehavior = .alertPanel + panel.contentViewController = controller + panel.delegate = self + panel.setAccessibilityIdentifier(model.identifier) + panel.setFrameOrigin(Self.origin(for: size)) + self.panel = panel + self.model = model + panel.orderFrontRegardless() + } + + public func close() { + panel?.delegate = nil + panel?.orderOut(nil) + panel = nil + } + + /// ⌘W answers the way Deny does. + public func windowShouldClose(_ sender: NSWindow) -> Bool { + model?.deny() + return false + } + + /// Horizontally centred in the upper fifth of the screen under the pointer, where macOS puts an alert. + private static func origin(for size: CGSize) -> CGPoint { + let mouse = NSEvent.mouseLocation + let screen = NSScreen.screens.first { NSMouseInRect(mouse, $0.frame, false) } ?? NSScreen.main + let visible = screen?.visibleFrame ?? CGRect(x: 0, y: 0, width: 1440, height: 900) + let top = visible.maxY - visible.height * 0.2 + return CGPoint(x: (visible.midX - size.width / 2).rounded(), y: (top - size.height).rounded()) + } +} + +/// A titled panel that can take the keyboard without activating its app. +private final class ApprovalPanel: NSPanel { + override var canBecomeKey: Bool { true } + override var canBecomeMain: Bool { false } +} diff --git a/src/macos/GenesisTools/Sources/Gate.swift b/src/macos/GenesisTools/Sources/Gate.swift index be5acc383b..4d86ad21ca 100644 --- a/src/macos/GenesisTools/Sources/Gate.swift +++ b/src/macos/GenesisTools/Sources/Gate.swift @@ -68,72 +68,6 @@ private func describeToken(_ kind: String?, provider: String) -> String { /// a little earlier and says why. private let gateDeadlineSeconds: Double = 170 -private func rememberLabel(_ seconds: Int) -> String { - if seconds <= 0 { - return "Allow once" - } - - if seconds % 86_400 == 0 { - let days = seconds / 86_400 - return "Allow for \(days) day\(days == 1 ? "" : "s")" - } - - if seconds % 3_600 == 0 { - let hours = seconds / 3_600 - return "Allow for \(hours) hour\(hours == 1 ? "" : "s")" - } - - return "Allow for \(seconds / 60) min" -} - -private func describeClient(_ client: GateClientInfo) -> String { - var lines: [String] = [] - - if let pid = client.pid { - lines.append("Process: pid \(pid)") - } else { - lines.append("Process: not identified (no pid)") - } - - if let executable = client.executable, !executable.isEmpty { - lines.append("Executable: \(executable)") - } else if client.pid != nil { - lines.append("Executable: not running under that pid") - } - - if let script = client.script, !script.isEmpty { - lines.append("Script: \(script)") - } - - if let command = client.command, !command.isEmpty, command != client.executable { - lines.append("Command: \(command)") - } - - if let cwd = client.cwd, !cwd.isEmpty { - lines.append("Working dir: \(cwd)") - } - - if let ancestors = client.ancestors, !ancestors.isEmpty { - let chain = ancestors.map { "\($0.command) (\($0.pid))" }.joined(separator: " ← ") - lines.append("Started by: \(chain)") - } - - if let injected = client.injected, !injected.isEmpty { - lines.append("⚠️ Extra code may be loaded: \(injected.joined(separator: ", "))") - } - - // A running pid that did not start the request is refused before this window opens, so an - // unverified client here is either a real ancestor whose identity is incomplete, or no pid. - if client.verified != true { - let why = client.isAncestor == true - ? "its binary or script could not be established" - : "no running pid was given" - lines.append("⚠️ Unverified: \(why). Allow once at most; it will not be remembered.") - } - - return lines.joined(separator: "\n") -} - private func gateEmit(decision: String, rememberSeconds: Int, method: String, reason: String) -> Never { logClick("gate.approve decision=\(decision) remember=\(rememberSeconds) method=\(method) reason=\(reason)") emitResult([ @@ -187,51 +121,119 @@ func authenticate(reason: String, completion: @escaping (_ method: String?, _ fa } } -func approveGate(_ params: GateApproveParams) { - let choices = (params.rememberChoicesSeconds?.isEmpty == false ? params.rememberChoicesSeconds : nil) ?? [0, 8 * 3_600, 7 * 86_400] - let accountLabel = params.account.label.map { "\(params.account.name) (\($0))" } ?? params.account.name +/// What the gate window shows, built from the CLI's request. Pure, so tests read it. +struct GateWindowContent { + var title: String + var purpose: String + var facts: [ApprovalFact] + var details: [ApprovalFact] + var warning: String? + var rememberChoices: [ApprovalRemember] + + static func rememberTitle(_ seconds: Int) -> String { + if seconds <= 0 { return "Just this once" } + if seconds % 86_400 == 0 { return "For \(Plural.count(seconds / 86_400, "day"))" } + if seconds % 3_600 == 0 { return "For \(Plural.count(seconds / 3_600, "hour"))" } + return "For \(seconds / 60) min" + } + + static func build(_ params: GateApproveParams, home: String = NSHomeDirectory()) -> GateWindowContent { + let client = params.client + let accountLabel = params.account.label.map { "\(params.account.name) (\($0))" } ?? params.account.name + let choices = (params.rememberChoicesSeconds?.isEmpty == false ? params.rememberChoicesSeconds : nil) ?? [0, 8 * 3_600, 7 * 86_400] + let program = client.script ?? client.executable + + var facts = [ApprovalFact("Account", "\(accountLabel) · \(params.provider)")] + facts.append(ApprovalFact("Program", program.map { "\(client.name) · \(VaultWindowContent.shortPath($0, home: home))" } ?? client.name, + help: [client.executable, client.script].compactMap { $0 }.joined(separator: " "))) + if let cwd = client.cwd, !cwd.isEmpty { + facts.append(ApprovalFact("Folder", VaultWindowContent.shortPath(cwd, home: home), help: cwd)) + } - DispatchQueue.main.asyncAfter(deadline: .now() + gateDeadlineSeconds) { - gateEmit(decision: "deny", rememberSeconds: 0, method: "none", reason: "no answer within \(Int(gateDeadlineSeconds))s") - } + var details = [ApprovalFact("Process", client.pid.map { "pid \($0)" } ?? "not identified (no pid)")] + if let executable = client.executable, !executable.isEmpty { + details.append(ApprovalFact("Executable", executable, monospaced: true)) + } - DispatchQueue.main.async { - NSApplication.shared.activate(ignoringOtherApps: true) + if let command = client.command, !command.isEmpty, command != client.executable { + details.append(ApprovalFact("Command", command, monospaced: true)) + } - let isApiKey = params.tokenKind == "api-key" - let alert = NSAlert() - alert.alertStyle = .warning - alert.messageText = isApiKey - ? "“\(params.client.name)” asks for the API KEY of the \(params.provider) account \(accountLabel)" - : "“\(params.client.name)” asks for the \(params.provider) token of \(accountLabel)" - alert.informativeText = describeClient(params.client) - + "\n\n" + describeToken(params.tokenKind, provider: params.provider) + " Touch ID confirms an allow." - - for seconds in choices { - alert.addButton(withTitle: rememberLabel(seconds)) + if let ancestors = client.ancestors, !ancestors.isEmpty { + details.append(ApprovalFact("Started by", ancestors.map { "\($0.command) (\($0.pid))" }.joined(separator: " ← "))) + } + + var warnings: [String] = [] + if let injected = client.injected, !injected.isEmpty { + warnings.append("Extra code may be loaded (\(injected.joined(separator: ", "))).") + } + + // A running pid that did not start the request is refused before this window opens, so an unverified client + // here is either a real ancestor whose identity is incomplete, or no pid. + if client.verified != true { + warnings.append(client.isAncestor == true + ? "Unverified: its binary or script could not be established. It will not be remembered." + : "Unverified: no running pid was given. It will not be remembered.") } - let deny = alert.addButton(withTitle: "Deny") - deny.keyEquivalent = "\u{1b}" + let isApiKey = params.tokenKind == "api-key" + return GateWindowContent( + title: isApiKey + ? "“\(client.name)” wants the \(params.provider) API key of \(params.account.name)" + : "“\(client.name)” wants the \(params.provider) token of \(params.account.name)", + purpose: describeToken(params.tokenKind, provider: params.provider), + facts: facts, + details: details, + warning: warnings.isEmpty ? nil : warnings.joined(separator: " "), + rememberChoices: choices.map { ApprovalRemember(seconds: $0, title: rememberTitle($0)) }) + } +} + +@MainActor private let gatePresenter = ApprovalPanelPresenter() + +func approveGate(_ params: GateApproveParams) { + let content = GateWindowContent.build(params) + let biometric = LAContext().canEvaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, error: nil) - let response = alert.runModal() - let index = response.rawValue - NSApplication.ModalResponse.alertFirstButtonReturn.rawValue + DispatchQueue.main.async { + let model = ApprovalDialogModel( + title: content.title, purpose: content.purpose, symbol: "person.badge.key.fill", facts: content.facts, + details: content.details, warning: content.warning, + footnote: "Every decision is written to the gate audit log (tools ai gate audit).", + rememberChoices: content.rememberChoices.count > 1 ? content.rememberChoices : [], + rememberNote: content.rememberChoices.count > 1 ? nil : "Only once", + allowTitle: biometric ? "Allow with Touch ID" : "Allow…", + deadline: Date().addingTimeInterval(gateDeadlineSeconds), identifier: "gate-approve") + + // The NSAlert this replaces ran a modal loop on the main queue, so this timer never fired there. + let deadline = DispatchWorkItem { + gateEmit(decision: "deny", rememberSeconds: 0, method: "none", reason: "no answer within \(Int(gateDeadlineSeconds))s") + } + DispatchQueue.main.asyncAfter(deadline: .now() + gateDeadlineSeconds, execute: deadline) - guard index >= 0, index < choices.count else { + model.onDeny = { + deadline.cancel() + gatePresenter.close() gateEmit(decision: "deny", rememberSeconds: 0, method: "none", reason: "denied in the approval window") } - let remember = choices[index] - let reason = isApiKey - ? "give “\(params.client.name)” the API key of the \(params.provider) account \(params.account.name)" - : "allow “\(params.client.name)” to use the \(params.provider) account \(params.account.name)" + model.onAllow = { choice in + deadline.cancel() + gatePresenter.close() + let remember = choice?.seconds ?? 0 + let reason = params.tokenKind == "api-key" + ? "give “\(params.client.name)” the API key of the \(params.provider) account \(params.account.name)" + : "allow “\(params.client.name)” to use the \(params.provider) account \(params.account.name)" + + authenticate(reason: reason) { method, failure in + guard let method else { + gateEmit(decision: "deny", rememberSeconds: 0, method: "none", reason: failure) + } - authenticate(reason: reason) { method, failure in - guard let method else { - gateEmit(decision: "deny", rememberSeconds: 0, method: "none", reason: failure) + gateEmit(decision: "allow", rememberSeconds: remember, method: method, reason: "") } - - gateEmit(decision: "allow", rememberSeconds: remember, method: method, reason: "") } + + gatePresenter.present(model) } } diff --git a/src/macos/GenesisTools/Sources/VaultUnlock.swift b/src/macos/GenesisTools/Sources/VaultUnlock.swift index c2c447f03e..fcde0c49d6 100644 --- a/src/macos/GenesisTools/Sources/VaultUnlock.swift +++ b/src/macos/GenesisTools/Sources/VaultUnlock.swift @@ -4,17 +4,23 @@ // // A `gt-` process needs the GenesisTools vault master key. Before this, each tool's own bun link read the // keychain item and macOS asked "gt-claude wants to use your confidential information" with nothing to judge. Now the -// tool starts this process and the app reads the item as itself (macOS asks once, naming GenesisTools). +// tool starts this process, and the app hands the key on after its own window. // // WHO comes from the kernel, never from the request: this process is the caller's child, so `getppid()` is the caller, -// and its executable, arguments, environment (the session ids), working folder and parent chain are read from there. -// WHY and WHAT come from the request, because only the caller knows them. A remembered approval for the same tool, +// and its executable, script, arguments, environment (the session ids), working folder and parent chain are read from +// there. WHY and WHAT come from the request, because only the caller knows them. A remembered approval for the same // executable and script skips the window; otherwise Touch ID confirms an allow. Every outcome goes to the gate's audit // log. A deny, a cancel, a failed authentication and a timeout all reply "deny" with a reason. +// +// Second version, after Martin's first look (2026-10-15 00:28, "this can be probably nicer", and "it wanted touchid +// and when i gave it it wanted password 2 times always"): a SwiftUI panel instead of an NSAlert (whose modal loop +// also kept the 170 s deadline from ever firing), `bun run