From 9780b14f72c0b06d005473d34b011a299f9d289b Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:05:21 -0400 Subject: [PATCH 01/18] chat: stop ADE overriding the user's Claude settings at flag tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADE passes its Claude settings to the Agent SDK at flag tier, which outranks every settings.json the SDK reads. Three keys were being sent unconditionally, so a value ADE invented always beat the user's own configuration: - outputStyle: resolved from the lane's settings.local.json with a `?? "Default"` fallback. "Default" is a real style, so a style configured in ~/.claude never applied to any ADE chat. The resolver also wrote that substituted value back into the session cache and read the cache first on the next build, which pinned it permanently once a session had started. - workflowSizeGuideline: hardcoded "medium", so the user's /config choice had no effect. ADE keeps supplying "medium" as its own default, but only while no settings file states one. - The user-tier root ignored CLAUDE_CONFIG_DIR, unlike the six other ADE modules that read Claude config, so a relocated config dir was invisible here. The rule: name a settings key only when ADE genuinely owns it, and otherwise leave it absent so the SDK's own local > project > user precedence resolves it. ADE already opts into that precedence via settingSources. enabledPlugins stays unconditional — the CLI merges it per plugin key rather than replacing the map. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/agentChatService.test.ts | 53 ++++++++++++++++- .../main/services/chat/agentChatService.ts | 38 +++++++++---- .../services/chat/claudeOutputStyles.test.ts | 44 ++++++++++++++ .../main/services/chat/claudeOutputStyles.ts | 57 ++++++++++++++++--- 4 files changed, 172 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 81e9f92e6..6131ab4e4 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -3508,8 +3508,13 @@ describe("createAgentChatService", () => { ])); expect(opts?.includeHookEvents).toBe(true); expect(opts?.promptSuggestions).toBe(true); + // No settings file names a style here, so ADE must not name one either: + // its settings land at flag tier, above every file the SDK reads, so an + // "outputStyle" key would override the user's global selection. + expect(opts?.settings).not.toHaveProperty("outputStyle"); expect(opts?.settings).toEqual(expect.objectContaining({ - outputStyle: "Default", + // ADE's own default, which applies only while no settings file states one. + workflowSizeGuideline: "medium", fastMode: false, enabledPlugins: expect.objectContaining({ "learning-output-style@claude-code-plugins": false, @@ -3520,6 +3525,52 @@ describe("createAgentChatService", () => { })); }); + it("passes the user's global output style through instead of pinning Default", async () => { + // The regression this guards: ADE substituted "Default" for "nothing is + // set" and passed it at flag tier, so a style configured in the user's + // settings.json never took effect in any ADE chat. + const userClaudeDir = path.join(tmpRoot, "user-claude-config"); + fs.mkdirSync(path.join(userClaudeDir, "output-styles"), { recursive: true }); + fs.writeFileSync( + path.join(userClaudeDir, "output-styles", "asd-ste100.md"), + ["---", "name: ASD-STE100", "description: Simplified Technical English", "---", "", "Write short sentences.", ""].join("\n"), + ); + fs.writeFileSync( + path.join(userClaudeDir, "settings.json"), + JSON.stringify({ outputStyle: "ASD-STE100", workflowSizeGuideline: "large" }), + ); + const previousConfigDir = process.env.CLAUDE_CONFIG_DIR; + process.env.CLAUDE_CONFIG_DIR = userClaudeDir; + + try { + vi.mocked(claudeSdkCreateSessionCompat).mockReturnValue({ + send: vi.fn(), + stream: vi.fn(async function* () { + return; + }), + close: vi.fn(), + sessionId: "sdk-session-user-output-style", + } as any); + + const { service } = createService(); + await service.createSession({ laneId: "lane-1", provider: "claude", model: "sonnet" }); + + await vi.waitFor(() => { + expect(claudeSdkCreateSessionCompat).toHaveBeenCalled(); + }); + + const opts = vi.mocked(claudeSdkCreateSessionCompat).mock.calls[0]?.[0] as { + settings?: { outputStyle?: string; workflowSizeGuideline?: string }; + } | undefined; + expect(opts?.settings?.outputStyle).toBe("ASD-STE100"); + // A user-stated guideline replaces ADE's default rather than losing to it. + expect(opts?.settings).not.toHaveProperty("workflowSizeGuideline"); + } finally { + if (previousConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = previousConfigDir; + } + }); + it("passes Claude fast mode through SDK flag settings for Opus sessions", async () => { vi.mocked(claudeSdkCreateSessionCompat).mockReturnValue({ send: vi.fn(), diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 08e24beee..5ea2f2ab9 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -70,6 +70,7 @@ import { discoverClaudePlugins, discoverClaudeOutputStyles, readClaudeOutputStyleSelection, + readClaudeWorkflowSizeGuideline, resolveClaudeOutputStyle, writeClaudeOutputStyleSelection, } from "./claudeOutputStyles"; @@ -28948,14 +28949,15 @@ export function createAgentChatService(args: { /** * Build stable Agent SDK query options from the managed session state. */ - const resolveManagedClaudeOutputStyle = (managed: ManagedChatSession): string => { - const requested = normalizePersistedOutputStyle(managed.session.claudeOutputStyle) - ?? readClaudeOutputStyleSelection(managed.laneWorktreePath); - const resolved = resolveClaudeOutputStyle(managed.laneWorktreePath, requested) - ?? resolveClaudeOutputStyle(managed.laneWorktreePath, "Default"); - const outputStyle = resolved?.name ?? "Default"; - managed.session.claudeOutputStyle = outputStyle; - return outputStyle; + const resolveManagedClaudeOutputStyle = (managed: ManagedChatSession): string | null => { + // Settings files first, session cache second: the cache is a display value we + // wrote ourselves last run, so consulting it first would pin whatever it holds + // and make an unset lane permanently ignore the user's global selection. + const requested = readClaudeOutputStyleSelection(managed.laneWorktreePath) + ?? normalizePersistedOutputStyle(managed.session.claudeOutputStyle); + const resolved = requested ? resolveClaudeOutputStyle(managed.laneWorktreePath, requested) : null; + managed.session.claudeOutputStyle = resolved?.name ?? null; + return resolved?.name ?? null; }; const buildClaudeQueryOptions = ( @@ -28980,6 +28982,10 @@ export function createAgentChatService(args: { }; const claudeExecutable = resolveClaudeCodeExecutable({ env: claudeEnv }); const outputStyle = resolveManagedClaudeOutputStyle(managed); + // ADE's preferred default, supplied only when no settings file states one. + const workflowSizeGuideline = readClaudeWorkflowSizeGuideline(managed.laneWorktreePath) + ? undefined + : "medium"; const bundledPluginPaths = claudeAgentSkillPluginRoots(claudeEnv); const pluginPaths = personalSession ? [] @@ -29006,11 +29012,16 @@ export function createAgentChatService(args: { // back. Workers/validators do real work and keep user MCP. strictMcpConfig still // permits the programmatic orchestration MCP server added below for bundled leads. ...((lightweight || isOrchestrationLeadSession(managed.session)) ? { strictMcpConfig: true } : {}), + // ADE's settings land at flag tier, above every settings.json the SDK reads. + // Only name a key ADE actually owns; anything else must stay absent so the + // SDK's own local > project > user precedence resolves it. `enabledPlugins` + // is safe to always send because the CLI merges it per plugin key rather + // than replacing the map. settings: { - outputStyle, + ...(outputStyle ? { outputStyle } : {}), enabledPlugins: CLAUDE_SESSION_DISABLED_PLUGINS, fastMode: sessionEffectiveFastMode(managed.session), - workflowSizeGuideline: "medium", + ...(workflowSizeGuideline ? { workflowSizeGuideline } : {}), }, ...(pluginPaths.length ? { plugins: pluginPaths.map((pluginPath) => ({ type: "local" as const, path: pluginPath })) } : {}), permissionMode: claudePermissionMode as any, @@ -31150,7 +31161,8 @@ export function createAgentChatService(args: { }; })(); const initialClaudeOutputStyle = effectiveProvider === "claude" - ? normalizePersistedOutputStyle(requestedClaudeOutputStyle) ?? readClaudeOutputStyleSelection(launchContext.laneWorktreePath) + ? normalizePersistedOutputStyle(requestedClaudeOutputStyle) + ?? readClaudeOutputStyleSelection(launchContext.laneWorktreePath) : null; const normalizedGoal = typeof requestedGoal === "string" && requestedGoal.trim().length @@ -45995,7 +46007,9 @@ export function createAgentChatService(args: { const requestedStyle = match[1]?.trim() ?? ""; managed.session.lastActivityAt = nowIso(); if (!requestedStyle.length) { - managed.session.claudeOutputStyle = managed.session.claudeOutputStyle ?? readClaudeOutputStyleSelection(managed.laneWorktreePath); + managed.session.claudeOutputStyle = managed.session.claudeOutputStyle + ?? readClaudeOutputStyleSelection(managed.laneWorktreePath) + ?? "Default"; emitChatEvent(managed, { type: "system_notice", noticeKind: "info", diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts index b3b39e1a9..664d3f34b 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts @@ -7,20 +7,29 @@ import { discoverClaudePlugins, discoverClaudeOutputStyles, readClaudeOutputStyleSelection, + readClaudeWorkflowSizeGuideline, resolveClaudeOutputStyle, writeClaudeOutputStyleSelection, } from "./claudeOutputStyles"; let tmpRoot: string; let homeRoot: string; +let previousClaudeConfigDir: string | undefined; beforeEach(() => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-claude-output-styles-test-")); homeRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-claude-output-styles-home-")); vi.spyOn(os, "homedir").mockReturnValue(homeRoot); + // The shared test setup points CLAUDE_CONFIG_DIR at its own temp dir, which now + // wins over homedir(). Aim it at this test's home so the user tier is the one + // these cases write to, and so no case can read the developer's real ~/.claude. + previousClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; + process.env.CLAUDE_CONFIG_DIR = path.join(homeRoot, ".claude"); }); afterEach(() => { + if (previousClaudeConfigDir === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = previousClaudeConfigDir; vi.restoreAllMocks(); fs.rmSync(tmpRoot, { recursive: true, force: true }); fs.rmSync(homeRoot, { recursive: true, force: true }); @@ -238,3 +247,38 @@ describe("discoverClaudePlugins", () => { expect(discoverClaudePlugins(tmpRoot).map((plugin) => plugin.name)).toEqual(["review-pack"]); }); }); + +describe("settings precedence", () => { + const writeSettings = (root: string, fileName: string, value: Record): void => { + const dir = path.join(root, ".claude"); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, fileName), JSON.stringify(value, null, 2)); + }; + + it("returns null rather than \"Default\" when no settings file names a style", () => { + // "Default" is a real style that suppresses a globally configured one. ADE + // passes its settings at flag tier, so a substituted default would silently + // override the user's ~/.claude selection in every lane. + expect(readClaudeOutputStyleSelection(tmpRoot)).toBeNull(); + }); + + it("falls back to the user's ~/.claude selection when the lane declares none", () => { + writeSettings(homeRoot, "settings.json", { outputStyle: "ASD-STE100" }); + expect(readClaudeOutputStyleSelection(tmpRoot)).toBe("ASD-STE100"); + }); + + it("prefers lane settings.local.json over lane settings.json over the user file", () => { + writeSettings(homeRoot, "settings.json", { outputStyle: "UserStyle" }); + writeSettings(tmpRoot, "settings.json", { outputStyle: "ProjectStyle" }); + expect(readClaudeOutputStyleSelection(tmpRoot)).toBe("ProjectStyle"); + + writeSettings(tmpRoot, "settings.local.json", { outputStyle: "LaneStyle" }); + expect(readClaudeOutputStyleSelection(tmpRoot)).toBe("LaneStyle"); + }); + + it("reads workflowSizeGuideline through the same ladder", () => { + expect(readClaudeWorkflowSizeGuideline(tmpRoot)).toBeNull(); + writeSettings(homeRoot, "settings.json", { workflowSizeGuideline: "large" }); + expect(readClaudeWorkflowSizeGuideline(tmpRoot)).toBe("large"); + }); +}); diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts index a251caf7d..c63cf2b70 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts @@ -208,10 +208,19 @@ function ancestorClaudeRoots(cwd: string): string[] { return roots; } +/** + * The user-level Claude root. `CLAUDE_CONFIG_DIR` relocates it wholesale — the + * CLI reads settings, styles and plugins from there instead of `~/.claude`, and + * every other ADE module that touches Claude config already honours it. + */ +function userClaudeRoot(): string { + const configured = process.env.CLAUDE_CONFIG_DIR?.trim(); + return configured?.length ? path.resolve(configured) : path.join(path.resolve(os.homedir()), ".claude"); +} + function claudeRootsByPrecedence(cwd: string): string[] { const roots: string[] = []; const seen = new Set(); - const home = path.resolve(os.homedir()); const addRoot = (root: string): void => { if (seen.has(root)) return; seen.add(root); @@ -219,7 +228,7 @@ function claudeRootsByPrecedence(cwd: string): string[] { }; for (const root of ancestorClaudeRoots(cwd)) addRoot(root); - addRoot(path.join(home, ".claude")); + addRoot(userClaudeRoot()); return roots; } @@ -343,7 +352,7 @@ export function discoverClaudeOutputStyles(cwd: string): AgentChatClaudeOutputSt for (const style of CLAUDE_BUILT_IN_OUTPUT_STYLES) add(style); const roots = claudeRootsByPrecedence(cwd); - const homeClaudeRoot = path.resolve(os.homedir(), ".claude"); + const homeClaudeRoot = userClaudeRoot(); const cwdClaudeRoot = path.resolve(cwd, ".claude"); for (const root of roots) { const resolvedRoot = path.resolve(root); @@ -370,8 +379,7 @@ export function claudeSettingsLocalPath(cwd: string): string { return path.join(cwd, ".claude", "settings.local.json"); } -export function readClaudeSettingsLocal(cwd: string): ClaudeSettingsLocal { - const settingsPath = claudeSettingsLocalPath(cwd); +function readClaudeSettingsFile(settingsPath: string): ClaudeSettingsLocal { try { const raw = fs.readFileSync(settingsPath, "utf8"); const parsed = JSON.parse(raw); @@ -383,8 +391,43 @@ export function readClaudeSettingsLocal(cwd: string): ClaudeSettingsLocal { } } -export function readClaudeOutputStyleSelection(cwd: string): string { - return maybeString(readClaudeSettingsLocal(cwd).outputStyle) ?? "Default"; +export function readClaudeSettingsLocal(cwd: string): ClaudeSettingsLocal { + return readClaudeSettingsFile(claudeSettingsLocalPath(cwd)); +} + +/** + * First value for `key` across the same settings files, in the same order, that + * the Agent SDK itself resolves with `settingSources: ["user", "project", "local"]`: + * lane `settings.local.json`, lane `settings.json`, each ancestor root, then + * `~/.claude`. Returns null when no file declares the key. + * + * ADE reads these only to decide whether it has anything to say. A key nobody + * declares must stay absent from the SDK options — ADE passes its settings at + * flag tier, which outranks every file, so substituting a default here silently + * overrides the user's global configuration. + */ +function readClaudeSettingsValue(cwd: string, key: string): string | null { + for (const root of claudeRootsByPrecedence(cwd)) { + for (const fileName of ["settings.local.json", "settings.json"]) { + const value = maybeString(readClaudeSettingsFile(path.join(root, fileName))[key]); + if (value) return value; + } + } + return null; +} + +/** + * The output style the user selected, or null when no settings file names one. + * Null means "ADE has no opinion" — never "Default", which is a real style that + * would suppress a globally configured one. + */ +export function readClaudeOutputStyleSelection(cwd: string): string | null { + return readClaudeSettingsValue(cwd, "outputStyle"); +} + +/** The workflow size guideline the user configured, or null when none is set. */ +export function readClaudeWorkflowSizeGuideline(cwd: string): string | null { + return readClaudeSettingsValue(cwd, "workflowSizeGuideline"); } export function writeClaudeOutputStyleSelection(cwd: string, outputStyle: string): string { From 64785970f2b4bae1ab211b860306ba7f82688717 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:57:14 -0400 Subject: [PATCH 02/18] codex: stop forcing service tier to "default" over the user's config.toml codexServiceTierArgs returned an explicit null whenever fast mode was not on, which includes every session where the user never touched the fast toggle. Verified against a live `codex app-server`, with service_tier = "priority" in config.toml: omit -> serviceTier = priority (the user's value survives) null -> serviceTier = default (the user's value is erased) and with no service_tier configured at all: omit -> no tier null -> "default" fast -> "priority" So null is a real downgrade rather than a neutral "no opinion", and ADE shows no service tier anywhere for the user to notice or undo it. Fast-off cannot mean "force default" either: fastMode is persisted only when true and rehydrated as `persisted?.fastMode === true`, so false is indistinguishable from never-set. Omitting is the only honest encoding of "ADE is not forcing a tier"; the app-server re-resolves per request. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/agentChatService.test.ts | 15 ++++++++------ .../main/services/chat/agentChatService.ts | 20 +++++++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 6131ab4e4..103e20576 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -28316,7 +28316,7 @@ describe("createAgentChatService", () => { })); }); - it("explicitly clears Codex service tier when fast mode is off", async () => { + it("omits Codex service tier when fast mode was never turned on", async () => { mockState.codexResponseOverrides.set("thread/start", (payload) => ({ thread: { id: "thread-default" }, serviceTier: (payload.params as { serviceTier?: unknown } | undefined)?.serviceTier ?? null, @@ -28337,17 +28337,20 @@ describe("createAgentChatService", () => { expect(mockState.codexRequestPayloads.some((payload) => payload.method === "turn/start")).toBe(true); }); + // Verified against a live app-server: omitting inherits the user's + // config.toml service_tier, while an explicit null forces "default". + // ADE has no service-tier UI, so it must not name the key at all. const threadStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "thread/start"); - expect((threadStartRequest?.params as { serviceTier?: unknown } | undefined)?.serviceTier).toBeNull(); + expect(threadStartRequest?.params).not.toHaveProperty("serviceTier"); const turnStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "turn/start"); - expect((turnStartRequest?.params as { serviceTier?: unknown } | undefined)?.serviceTier).toBeNull(); + expect(turnStartRequest?.params).not.toHaveProperty("serviceTier"); const summary = await service.getSessionSummary(session.id); expect(summary?.fastMode).toBe(false); expect(summary?.codexServiceTier).toBeNull(); expect(readPersistedChatState(session.id).codexServiceTier).toBeNull(); }); - it("preserves fast mode selection on unsupported Codex models while sending standard tier", async () => { + it("preserves fast mode selection on unsupported Codex models without naming a tier", async () => { const { service } = createService(); const session = await service.createSession({ laneId: "lane-1", @@ -28366,9 +28369,9 @@ describe("createAgentChatService", () => { }); const threadStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "thread/start"); - expect((threadStartRequest?.params as { serviceTier?: unknown } | undefined)?.serviceTier).toBeNull(); + expect(threadStartRequest?.params).not.toHaveProperty("serviceTier"); const turnStartRequest = mockState.codexRequestPayloads.find((payload) => payload.method === "turn/start"); - expect((turnStartRequest?.params as { serviceTier?: unknown } | undefined)?.serviceTier).toBeNull(); + expect(turnStartRequest?.params).not.toHaveProperty("serviceTier"); expect((await service.getSessionSummary(session.id))?.fastMode).toBe(true); }); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 5ea2f2ab9..94e1b7203 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -3655,10 +3655,22 @@ function sessionEffectiveFastMode( return session.fastMode === true && sessionSupportsFastMode(session, catalog); } -function codexServiceTierArgs(session: AgentChatSession): { serviceTier: CodexServiceTier | null } { - // JSON-RPC needs an explicit null to clear any app-server/config default. - const serviceTier = session.fastMode === true && sessionSupportsCodexServiceTier(session) ? "fast" : null; - return { serviceTier }; +function codexServiceTierArgs(session: AgentChatSession): { serviceTier?: CodexServiceTier | null } { + if (session.fastMode === true && sessionSupportsCodexServiceTier(session)) { + return { serviceTier: "fast" }; + } + // Verified against a live app-server on thread/start: omitting the key inherits + // the user's config.toml (service_tier = "priority" -> "priority"; unset -> no + // tier), while an explicit null reports "default" in both cases. null is + // therefore a real downgrade, not a neutral "no opinion", and ADE has no UI + // showing service tier for the user to notice or undo it. + // + // Fast-off cannot mean "force default" either: fastMode is persisted only when + // true and rehydrated as `persisted?.fastMode === true`, so `false` is + // indistinguishable from never-set. Omitting is the only honest encoding of + // "ADE is not forcing a tier" — the app-server re-resolves per request, so a + // turn sent after the toggle goes off inherits the config again. + return {}; } function codexThreadConfigArgs( From 7051bc374299813a5fae3a6aeaa00e862a637bd6 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:57:23 -0400 Subject: [PATCH 03/18] providers: read config from the directory each CLI actually uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every provider CLI has an env var that relocates its config directory, and ADE ignored some of them — so ADE read one directory while the process it spawned read another, inside a single session. Confirmed with a sentinel custom model: with FACTORY_HOME_OVERRIDE set, `droid` lists the override home's models while droidModelsDiscovery read the real home's. The overrides do not share a shape, which is why this is a helper rather than a find-and-replace: - CLAUDE_CONFIG_DIR and CODEX_HOME name the config directory itself. - FACTORY_HOME_OVERRIDE replaces the HOME that ".factory" is appended to (`join($R(), ".factory")` in the droid v0.70.0 binary, where $R() is `process.env.FACTORY_HOME_OVERRIDE || homedir()`). Read paths only; no behavior changes for anyone without these vars set. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/droidModelsDiscovery.ts | 3 +- .../externalSessions/discoverDroid.ts | 3 +- .../providerSessionHandles.ts | 10 +++-- .../src/main/services/pty/ptyService.ts | 9 ++-- .../services/shared/providerConfigHomes.ts | 45 +++++++++++++++++++ 5 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 apps/desktop/src/main/services/shared/providerConfigHomes.ts diff --git a/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts b/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts index fc7340b04..08dee172b 100644 --- a/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts +++ b/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts @@ -8,6 +8,7 @@ import { type ModelDescriptor, } from "../../../shared/modelRegistry"; import { spawnAsync } from "../shared/utils"; +import { factoryConfigHome } from "../shared/providerConfigHomes"; export type DroidExecHelpModelRow = { id: string; @@ -319,7 +320,7 @@ function getCachedDroidModels(droidPathForRevalidate?: string | null): DroidExec */ async function readFactoryConfigCustomModels(): Promise { try { - const configPath = join(homedir(), ".factory", "config.json"); + const configPath = join(factoryConfigHome(), "config.json"); const raw = await readFile(configPath, "utf-8"); const parsed = JSON.parse(raw) as Record; const customModels = parsed.custom_models; diff --git a/apps/desktop/src/main/services/externalSessions/discoverDroid.ts b/apps/desktop/src/main/services/externalSessions/discoverDroid.ts index 8f2a92f2a..2d3f5091a 100644 --- a/apps/desktop/src/main/services/externalSessions/discoverDroid.ts +++ b/apps/desktop/src/main/services/externalSessions/discoverDroid.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { factoryConfigHome } from "../shared/providerConfigHomes"; import { asEpochMs, asRecord, @@ -81,7 +82,7 @@ export async function discoverDroidSessions( args: ExternalSessionDiscoveryArgs = {}, ): Promise { const limit = normalizeExternalSessionLimit(args.limit); - const sessionsDir = path.join(resolveHomeDir(args), ".factory", "sessions"); + const sessionsDir = path.join(factoryConfigHome({ ...process.env, HOME: resolveHomeDir(args) }), "sessions"); const lookupId = args.sessionId?.trim() || null; // A session id can be written under more than one escaped cwd; keeping one // candidate per id stops duplicates from spending the read budget twice and diff --git a/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts b/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts index 5e92426a4..4f770daea 100644 --- a/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts +++ b/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts @@ -4,6 +4,7 @@ import { resolveHomeDir } from "./discoveryUtils"; import { piSessionRootForEnvironment } from "../chat/piSessionStore"; import { isPathInside, pathKey } from "../shared/pathCompare"; import type { ExternalSessionProvider } from "../../../shared/types/externalSessions"; +import { claudeConfigHome, codexConfigHome, factoryConfigHome } from "../shared/providerConfigHomes"; export type ProviderSessionHandle = { provider: ExternalSessionProvider; @@ -94,10 +95,13 @@ export function providerSessionRoots(args: { const xdgData = typeof env.XDG_DATA_HOME === "string" && env.XDG_DATA_HOME.trim() ? env.XDG_DATA_HOME.trim() : path.join(homeDir, ".local", "share"); + // Each provider's config-dir override has its own shape; providerConfigHomes + // keeps them straight so ADE reads the same directory the CLI writes. + const providerEnv = { ...env, HOME: homeDir }; return [ - { provider: "claude", root: path.join(homeDir, ".claude", "projects") }, - { provider: "codex", root: path.join(homeDir, ".codex", "sessions") }, - { provider: "droid", root: path.join(homeDir, ".factory", "sessions") }, + { provider: "claude", root: path.join(claudeConfigHome(providerEnv), "projects") }, + { provider: "codex", root: path.join(codexConfigHome(providerEnv), "sessions") }, + { provider: "droid", root: path.join(factoryConfigHome(providerEnv), "sessions") }, { provider: "cursor", root: path.join(homeDir, ".cursor", "chats") }, { provider: "cursor", root: path.join(homeDir, ".cursor", "projects") }, { provider: "opencode", root: path.join(xdgData, "opencode") }, diff --git a/apps/desktop/src/main/services/pty/ptyService.ts b/apps/desktop/src/main/services/pty/ptyService.ts index bec9b9b46..598264ce6 100644 --- a/apps/desktop/src/main/services/pty/ptyService.ts +++ b/apps/desktop/src/main/services/pty/ptyService.ts @@ -135,6 +135,7 @@ import { claudeAgentSkillPluginRoots } from "../skills/agentSkillRuntimeService" import { stripAnsi } from "../../utils/ansiStrip"; import { summarizeTerminalSession } from "../../utils/sessionSummary"; import { derivePreviewFromChunk, type PreviewCursorState } from "../../utils/terminalPreview"; +import { codexConfigHome, factoryConfigHome } from "../shared/providerConfigHomes"; import { clearTuiWaitingInput, createTuiMarkerState, @@ -3207,7 +3208,7 @@ export function createPtyService({ } function readCodexThreadNameFromIndex(codexSessionId: string): string | null { - const indexPath = path.join(os.homedir(), ".codex", "session_index.jsonl"); + const indexPath = path.join(codexConfigHome(), "session_index.jsonl"); const text = readFileSuffix(indexPath, CODEX_THREAD_NAME_SCAN_BYTES); if (!text) return null; const lines = text.split(/\r?\n/).filter(Boolean); @@ -3249,7 +3250,7 @@ export function createPtyService({ ownershipOriginator?: string | null; }): CodexStorageSessionMatch | null => { try { - const sessionsBase = path.join(os.homedir(), ".codex", "sessions"); + const sessionsBase = path.join(codexConfigHome(), "sessions"); if (!fs.existsSync(sessionsBase)) return null; const now = new Date(); @@ -3384,7 +3385,7 @@ export function createPtyService({ maxStartDeltaMs?: number; }): string | null => { try { - const droidSessionsDir = path.join(os.homedir(), ".factory", "sessions"); + const droidSessionsDir = path.join(factoryConfigHome(), "sessions"); if (!fs.existsSync(droidSessionsDir)) return null; const projectEntries = fs.readdirSync(droidSessionsDir, { withFileTypes: true }) .filter((entry) => entry.isDirectory()); @@ -3944,7 +3945,7 @@ export function createPtyService({ ): void => { const startedAtMs = Date.parse(startedAt); const startedAtFinite = Number.isFinite(startedAtMs) ? startedAtMs : null; - const sessionsBase = path.join(os.homedir(), ".codex", "sessions"); + const sessionsBase = path.join(codexConfigHome(), "sessions"); let captured = false; const watchers: Array<{ close: () => void }> = []; const timers = new Set(); diff --git a/apps/desktop/src/main/services/shared/providerConfigHomes.ts b/apps/desktop/src/main/services/shared/providerConfigHomes.ts new file mode 100644 index 000000000..6654e7ba8 --- /dev/null +++ b/apps/desktop/src/main/services/shared/providerConfigHomes.ts @@ -0,0 +1,45 @@ +import os from "node:os"; +import path from "node:path"; + +/** + * Where each provider CLI keeps its user-level config. + * + * Every one of these has an env override that the provider's own binary honours, + * and the overrides do NOT share a shape — `CODEX_HOME` and `CLAUDE_CONFIG_DIR` + * name the config directory itself, while `FACTORY_HOME_OVERRIDE` replaces the + * HOME that `.factory` is then appended to. Hardcoding `~/.codex` or `~/.factory` + * makes ADE read a different directory than the process it spawns, so ADE and the + * CLI disagree about the user's configuration inside a single session. + */ + +function home(env: NodeJS.ProcessEnv): string { + const configured = env.HOME?.trim() || env.USERPROFILE?.trim(); + return configured?.length ? path.resolve(configured) : path.resolve(os.homedir()); +} + +function trimmed(value: string | undefined): string | null { + const next = value?.trim(); + return next?.length ? next : null; +} + +/** `CLAUDE_CONFIG_DIR` names the config directory itself. */ +export function claudeConfigHome(env: NodeJS.ProcessEnv = process.env): string { + const configured = trimmed(env.CLAUDE_CONFIG_DIR); + return configured ? path.resolve(configured) : path.join(home(env), ".claude"); +} + +/** `CODEX_HOME` names the config directory itself, not the parent. */ +export function codexConfigHome(env: NodeJS.ProcessEnv = process.env): string { + const configured = trimmed(env.CODEX_HOME); + return configured ? path.resolve(configured) : path.join(home(env), ".codex"); +} + +/** + * `FACTORY_HOME_OVERRIDE` replaces the HOME directory; Droid appends `.factory` + * to it (`join($R(), ".factory")` in the v0.70.0 binary, where `$R()` is + * `process.env.FACTORY_HOME_OVERRIDE || homedir()`). + */ +export function factoryConfigHome(env: NodeJS.ProcessEnv = process.env): string { + const configured = trimmed(env.FACTORY_HOME_OVERRIDE); + return path.join(configured ? path.resolve(configured) : home(env), ".factory"); +} From 6ca21e829cb939c3cb15323774faca8293bb17fc Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:05:43 -0400 Subject: [PATCH 04/18] cursor: stop switching off a sandbox policy the user configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADE sent `sandboxOptions: { enabled: local.sandboxEnabled }` unconditionally. In the vendored SDK an explicit `false` and an absent key are not equivalent: if (!1 === n?.enabled) return { defaultSandboxPolicy: { type: "insecure_none" } } const o = Q(r) ? r : (!0 === n?.enabled ? Y("workspace_readwrite", ...) : void 0) `false` returns before `perUserSandboxPolicy` — the user's ~/.cursor/sandbox.json — is ever read. ADE sent `false` for agent mode, so a user who wrote a Cursor sandbox policy had it silently switched off. The SDK's own error text ("remove ~/.cursor/sandbox.json to run without sandboxing") shows that file is meant to be authoritative. A boolean cannot express this, so the policy layer now states a directive: enable — ask/plan. ADE asks for a sandbox; a user policy still wins. disable — full access. No sandbox, including for a user who wrote a policy, because full access means full access. Also the retry after a ConfigurationError, where the environment cannot sandbox at all and the alternative is a hard failure. inherit — agent mode. ADE has no sandbox UI here, so it says nothing and the user's file decides. The retry guard now keys off the error and the not-yet-downgraded flag rather than off whether ADE asked for the sandbox, because with "inherit" the unsupported-environment error can now surface through the user's policy instead of ADE's request. The permission fingerprint tracks the directive, since "disable" and "inherit" share a false boolean but produce different options. Note the SDK only loads any sandbox policy when an apiKey is present, so this affects users with a Cursor key configured in ADE or CURSOR_API_KEY set. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/cursorSdkPolicy.test.ts | 32 +++++++++++++++++++ .../src/main/services/chat/cursorSdkPolicy.ts | 27 +++++++++++++++- .../src/main/services/chat/cursorSdkWorker.ts | 12 +++++-- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts b/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts index a2b4c32f2..3d9497091 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts @@ -434,3 +434,35 @@ describe("Cursor SDK policy", () => { expect(evaluateCursorSdkHook({ request: shell, policy, laneRoot })).toBe("allow"); }); }); + +describe("cursor sandbox directive", () => { + const directiveFor = (mode: string, sandboxSupported = true): string => + buildCursorSdkLocalRunOptions( + resolveCursorSdkPolicy({ cursorModeId: mode }), + { sandboxSupported }, + ).sandboxDirective; + + it("asks for a sandbox in the read-only modes", () => { + expect(directiveFor("ask")).toBe("enable"); + expect(directiveFor("plan")).toBe("enable"); + }); + + it("says nothing in agent mode so the user's sandbox.json decides", () => { + // An explicit false would return insecure_none without ever reading the + // user's policy file. ADE has no sandbox UI for this mode, so it must not + // state an opinion either way. + expect(directiveFor("agent")).toBe("inherit"); + }); + + it("disables the sandbox outright for full access", () => { + // Full access means no sandbox, including for a user who wrote a policy. + expect(directiveFor("full-auto")).toBe("disable"); + }); + + it("disables the sandbox when the environment cannot provide one", () => { + // The retry after a ConfigurationError: the alternative is a hard failure, + // so this is the one case where "false" is the honest answer. + expect(directiveFor("ask", false)).toBe("disable"); + expect(directiveFor("agent", false)).toBe("disable"); + }); +}); diff --git a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts index d0d9598f9..2e43e8412 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts @@ -60,12 +60,28 @@ export const CURSOR_SDK_READONLY_TOOLS = ["read", "grep", "glob", "ls"] as const export type CursorSdkReadonlyTool = (typeof CURSOR_SDK_READONLY_TOOLS)[number]; +/** + * What ADE has to say about the Cursor sandbox, which is not a boolean. + * + * The SDK treats an explicit `false` and an absent key differently: `false` + * returns `insecure_none` without ever reading the user's ~/.cursor/sandbox.json, + * while absent lets that file decide. So ADE needs three states, not two. + * + * - "enable" — ADE asks for a sandbox (ask/plan). A user policy still wins. + * - "disable" — full access means no sandbox, even for a user who wrote a policy. + * Also used after a ConfigurationError, where the environment + * cannot sandbox and the alternative is a hard failure. + * - "inherit" — ADE has no opinion. The user's sandbox.json decides. + */ +export type CursorSdkSandboxDirective = "enable" | "disable" | "inherit"; + export type CursorSdkLocalRunOptions = { mode: CursorSdkAgentMode; tools?: string[]; disallowedTools?: string[]; autoReview: boolean; sandboxEnabled: boolean; + sandboxDirective: CursorSdkSandboxDirective; }; /** @@ -86,13 +102,22 @@ export function buildCursorSdkLocalRunOptions( policy: CursorSdkPermissionPolicy, args?: { sandboxSupported?: boolean }, ): CursorSdkLocalRunOptions { - const sandboxEnabled = policy.sandbox === "cursor-native" && args?.sandboxSupported !== false; + const sandboxSupported = args?.sandboxSupported !== false; + const sandboxEnabled = policy.sandbox === "cursor-native" && sandboxSupported; + const sandboxDirective: CursorSdkSandboxDirective = !sandboxSupported + ? "disable" + : policy.sandbox === "cursor-native" + ? "enable" + : policy.sandbox === "off" + ? "disable" + : "inherit"; return { mode: cursorSdkLocalAgentMode(policy), ...(policy.tools?.length ? { tools: [...policy.tools] } : {}), ...(policy.disallowedTools?.length ? { disallowedTools: [...policy.disallowedTools] } : {}), autoReview: policy.autoReview, sandboxEnabled, + sandboxDirective, }; } diff --git a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts index 37c227499..dc7ca9787 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts @@ -229,7 +229,7 @@ function localPermissionFingerprint(policy: CursorSdkPermissionPolicy): string { tools: local.tools ?? null, disallowedTools: local.disallowedTools ?? null, autoReview: local.autoReview, - sandboxEnabled: local.sandboxEnabled, + sandboxDirective: local.sandboxDirective, }); } @@ -245,7 +245,13 @@ function buildLocalAgentOptions(init: CursorSdkWorkerInit): AgentOptionsWithAdeM local: { cwd: init.laneRoot, settingSources: cursorSdkSettingSources(init.policy), - sandboxOptions: { enabled: local.sandboxEnabled }, + // `false` and absent are not the same thing here. An explicit `false` + // returns `insecure_none` without ever reading the user's + // ~/.cursor/sandbox.json, so it is a deliberate statement rather than a + // neutral default. See CursorSdkSandboxDirective for the three cases. + ...(local.sandboxDirective === "inherit" + ? {} + : { sandboxOptions: { enabled: local.sandboxDirective === "enable" } }), autoReview: local.autoReview, enableAgentRetries: true, }, @@ -273,7 +279,7 @@ async function applyLocalAgentOptions(): Promise { lastLocalPermissionFingerprint = fingerprint; return options; } catch (error) { - if (!options.local?.sandboxOptions?.enabled || !isCursorSdkSandboxUnsupportedError(error)) { + if (!isCursorSdkSandboxUnsupportedError(error) || !sandboxSupported) { throw error; } sandboxSupported = false; From 3ec6093c1a7f2947922c3cae0656691f23bf93f3 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:13:46 -0400 Subject: [PATCH 05/18] droid: inherit autonomy from the user's settings, and fix the dead model read-back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects, found by driving the real @factory/droid-sdk against synthetic FACTORY_HOME_OVERRIDE homes. 1. ADE always stated autonomyLevel and interactionMode, so the user's ~/.factory/settings.json never applied. Same call, two homes differing only in settings.json: home A (model + spec/high configured) -> gemini-3-flash-preview / spec / high home B (empty settings.json) -> claude-opus-4-6 / auto / off Omission resolves from the user's file, per key. Any value ADE states outranks it. ADE's fallback was "auto-low", which permits file edits, while Droid's documented default is autonomyLevel "off" — read-only. So ADE was handing out write access the CLI would not, and then writing that invented value into the session record where it was read back first on the next launch and pinned, exactly as in the Claude output-style bug. The SDK path now says nothing when the user picked no mode, which is what the terminal path already did — droidSettingsJson omits sessionDefaultSettings when permissionMode is null. A chosen mode, plan, and orchestration leads all still state both keys. Keys are OMITTED, never nulled: an explicit null neither clears the key nor restores the default, it wedges the Droid RPC for 30 seconds. 2. buildReady read `initResult.currentModelId`, which does not exist — the SDK reports resolved settings under `initResult.settings`. It always evaluated to null, so applyDroidSdkReadyState's adoption branch had never fired and ADE could never learn what model Droid actually chose. Now reads initResult.settings.modelId. Also fixes providerConfigHomes to resolve its base from the named `homedir` import: test suites mock node:os by spreading the real module, so a default import kept the real homedir and read the developer's own ~/.factory. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../main/services/chat/agentChatService.ts | 51 ++++++++++++++++--- .../main/services/chat/droidSdkProtocol.ts | 11 +++- .../src/main/services/chat/droidSdkWorker.ts | 21 ++++++-- .../externalSessions/discoverDroid.ts | 2 +- .../providerSessionHandles.ts | 8 +-- .../services/shared/providerConfigHomes.ts | 31 ++++++----- 6 files changed, 93 insertions(+), 31 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 94e1b7203..66e2950a6 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -7128,14 +7128,31 @@ function resolveSessionOpenCodePermissionMode( ?? fallback; } -function resolveSessionDroidPermissionMode( +/** + * The Droid permission mode the user actually chose, or null when they have + * chosen nothing. + * + * Droid has no "use my config" mode — cliLaunch rejects config-toml for it — so + * null is the only way ADE can express "no opinion", and it matters: a live probe + * showed omitting these keys resolves them from the user's + * ~/.factory/settings.json, per key, while any value ADE states outranks that + * file. Droid's own documented default is autonomyLevel "off" (read-only), so a + * substituted fallback here hands out write access the CLI would not. + */ +function resolveSessionDroidPermissionModeOrNull( session: Pick, - fallback: AgentChatDroidPermissionMode, -): AgentChatDroidPermissionMode { +): AgentChatDroidPermissionMode | null { return session.droidPermissionMode ?? legacyPermissionModeToDroidPermissionMode(session.permissionMode) ?? legacyOpenCodePermissionModeToDroidPermissionMode(session.opencodePermissionMode) - ?? fallback; + ?? null; +} + +function resolveSessionDroidPermissionMode( + session: Pick, + fallback: AgentChatDroidPermissionMode, +): AgentChatDroidPermissionMode { + return resolveSessionDroidPermissionModeOrNull(session) ?? fallback; } function applyLocalHarnessPermissionMode(args: { @@ -7406,7 +7423,12 @@ function normalizeSessionNativePermissionControls( session.interactionMode = orchestrationMode ?? (session.interactionMode === "plan" || session.permissionMode === "plan" ? "plan" : "default"); - session.droidPermissionMode = resolveSessionDroidPermissionMode(session, "auto-low"); + // Materialising a fallback here would be read back as a real choice on the + // next launch and pin it forever, which is what made the equivalent Claude + // bug durable. Absence has to stay absent. + const chosenDroidMode = resolveSessionDroidPermissionModeOrNull(session); + if (chosenDroidMode) session.droidPermissionMode = chosenDroidMode; + else delete session.droidPermissionMode; delete session.claudePermissionMode; delete session.codexApprovalPolicy; delete session.codexSandbox; @@ -35097,14 +35119,27 @@ export function createAgentChatService(args: { // AGI (orchestrator) is a Droid-specific permission mode, not part of the // generic interaction-mode enum — resolve it from droidPermissionMode and // let it win over the plan→spec mapping. + const chosenMode = resolveSessionDroidPermissionModeOrNull(managed.session); + const planRequested = managed.session.interactionMode === "plan" + || managed.session.permissionMode === "plan"; + // Say nothing when the user picked nothing, so Droid resolves autonomy from + // their own settings.json exactly as the terminal path already does — + // droidSettingsJson omits sessionDefaultSettings when permissionMode is null. + const statesAutonomy = chosenMode !== null + || planRequested + || isOrchestrationLeadSession(managed.session); const interactionMode: DroidSdkSessionSettings["interactionMode"] = - resolveSessionDroidPermissionMode(managed.session, "auto-low") === "agi" + chosenMode === "agi" ? "agi" : resolveDroidSdkInteractionMode(managed.session); return { modelId, - autonomyLevel: resolveDroidSdkAutonomyLevel(managed.session), - interactionMode, + ...(statesAutonomy + ? { + autonomyLevel: resolveDroidSdkAutonomyLevel(managed.session), + interactionMode, + } + : {}), // Droid's own editor/terminal tools live outside ADE's toolset, so a lead // has to have them withheld natively as well. ...(isOrchestrationLeadSession(managed.session) diff --git a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts index 8cc554935..9d3dc686e 100644 --- a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts +++ b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts @@ -20,8 +20,15 @@ export type DroidSdkReasoningEffort = export type DroidSdkSessionSettings = { modelId: string; - autonomyLevel: DroidSdkAutonomyLevel; - interactionMode: DroidSdkInteractionMode; + /** + * Omitted when the user has chosen no ADE permission mode, so Droid resolves + * autonomy from their own ~/.factory/settings.json. Both keys are optional in + * the SDK, and omission resolves per key — a live probe confirmed an omitted + * key falls through to the user's file while any stated value outranks it. + * Never send null: an explicit null wedges the Droid RPC for 30 seconds. + */ + autonomyLevel?: DroidSdkAutonomyLevel; + interactionMode?: DroidSdkInteractionMode; reasoningEffort?: DroidSdkReasoningEffort | null; specModeModelId?: string | null; specModeReasoningEffort?: DroidSdkReasoningEffort | null; diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index 6b45e294b..e3c12e341 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -235,14 +235,29 @@ function normalizeAvailableModels(initResult: unknown): DroidSdkReady["available }); } +/** + * The model Droid actually resolved for this session. + * + * `initResult.currentModelId` does not exist — @factory/droid-sdk reports the + * resolved settings under `initResult.settings`, so the old read was dead code + * that always produced null, and every caller downstream silently fell back to + * ADE's own value instead of adopting Droid's. + */ +function readResolvedModelId(initResult: unknown): string | null { + const record = initResult && typeof initResult === "object" ? initResult as Record : null; + const settings = record?.settings && typeof record.settings === "object" + ? record.settings as Record + : null; + const modelId = typeof settings?.modelId === "string" ? settings.modelId.trim() : ""; + return modelId.length ? modelId : null; +} + function buildReady(): DroidSdkReady { if (!session) throw new Error("Droid SDK worker is not initialized."); const initResult = session.initResult as unknown; - const record = initResult && typeof initResult === "object" ? initResult as Record : null; - const currentModelId = typeof record?.currentModelId === "string" ? record.currentModelId : null; return { sessionId: session.sessionId, - currentModelId, + currentModelId: readResolvedModelId(initResult), availableModels: normalizeAvailableModels(initResult), }; } diff --git a/apps/desktop/src/main/services/externalSessions/discoverDroid.ts b/apps/desktop/src/main/services/externalSessions/discoverDroid.ts index 2d3f5091a..9144bdc97 100644 --- a/apps/desktop/src/main/services/externalSessions/discoverDroid.ts +++ b/apps/desktop/src/main/services/externalSessions/discoverDroid.ts @@ -82,7 +82,7 @@ export async function discoverDroidSessions( args: ExternalSessionDiscoveryArgs = {}, ): Promise { const limit = normalizeExternalSessionLimit(args.limit); - const sessionsDir = path.join(factoryConfigHome({ ...process.env, HOME: resolveHomeDir(args) }), "sessions"); + const sessionsDir = path.join(factoryConfigHome({ homeDir: resolveHomeDir(args) }), "sessions"); const lookupId = args.sessionId?.trim() || null; // A session id can be written under more than one escaped cwd; keeping one // candidate per id stops duplicates from spending the read budget twice and diff --git a/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts b/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts index 4f770daea..be00dab91 100644 --- a/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts +++ b/apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts @@ -97,11 +97,11 @@ export function providerSessionRoots(args: { : path.join(homeDir, ".local", "share"); // Each provider's config-dir override has its own shape; providerConfigHomes // keeps them straight so ADE reads the same directory the CLI writes. - const providerEnv = { ...env, HOME: homeDir }; + const providerHome = { env, homeDir }; return [ - { provider: "claude", root: path.join(claudeConfigHome(providerEnv), "projects") }, - { provider: "codex", root: path.join(codexConfigHome(providerEnv), "sessions") }, - { provider: "droid", root: path.join(factoryConfigHome(providerEnv), "sessions") }, + { provider: "claude", root: path.join(claudeConfigHome(providerHome), "projects") }, + { provider: "codex", root: path.join(codexConfigHome(providerHome), "sessions") }, + { provider: "droid", root: path.join(factoryConfigHome(providerHome), "sessions") }, { provider: "cursor", root: path.join(homeDir, ".cursor", "chats") }, { provider: "cursor", root: path.join(homeDir, ".cursor", "projects") }, { provider: "opencode", root: path.join(xdgData, "opencode") }, diff --git a/apps/desktop/src/main/services/shared/providerConfigHomes.ts b/apps/desktop/src/main/services/shared/providerConfigHomes.ts index 6654e7ba8..ec9d66db3 100644 --- a/apps/desktop/src/main/services/shared/providerConfigHomes.ts +++ b/apps/desktop/src/main/services/shared/providerConfigHomes.ts @@ -1,4 +1,4 @@ -import os from "node:os"; +import { homedir } from "node:os"; import path from "node:path"; /** @@ -10,11 +10,16 @@ import path from "node:path"; * HOME that `.factory` is then appended to. Hardcoding `~/.codex` or `~/.factory` * makes ADE read a different directory than the process it spawns, so ADE and the * CLI disagree about the user's configuration inside a single session. + * + * `homeDir` is for callers that already resolved a home of their own; everything + * else stays on `homedir()` so this matches how ADE resolved these paths + * before, and so tests that stub `os.homedir()` keep working. */ -function home(env: NodeJS.ProcessEnv): string { - const configured = env.HOME?.trim() || env.USERPROFILE?.trim(); - return configured?.length ? path.resolve(configured) : path.resolve(os.homedir()); +type HomeArg = { env?: NodeJS.ProcessEnv; homeDir?: string }; + +function baseHome(args: HomeArg): string { + return path.resolve(args.homeDir?.trim().length ? args.homeDir : homedir()); } function trimmed(value: string | undefined): string | null { @@ -23,15 +28,15 @@ function trimmed(value: string | undefined): string | null { } /** `CLAUDE_CONFIG_DIR` names the config directory itself. */ -export function claudeConfigHome(env: NodeJS.ProcessEnv = process.env): string { - const configured = trimmed(env.CLAUDE_CONFIG_DIR); - return configured ? path.resolve(configured) : path.join(home(env), ".claude"); +export function claudeConfigHome(args: HomeArg = {}): string { + const configured = trimmed((args.env ?? process.env).CLAUDE_CONFIG_DIR); + return configured ? path.resolve(configured) : path.join(baseHome(args), ".claude"); } /** `CODEX_HOME` names the config directory itself, not the parent. */ -export function codexConfigHome(env: NodeJS.ProcessEnv = process.env): string { - const configured = trimmed(env.CODEX_HOME); - return configured ? path.resolve(configured) : path.join(home(env), ".codex"); +export function codexConfigHome(args: HomeArg = {}): string { + const configured = trimmed((args.env ?? process.env).CODEX_HOME); + return configured ? path.resolve(configured) : path.join(baseHome(args), ".codex"); } /** @@ -39,7 +44,7 @@ export function codexConfigHome(env: NodeJS.ProcessEnv = process.env): string { * to it (`join($R(), ".factory")` in the v0.70.0 binary, where `$R()` is * `process.env.FACTORY_HOME_OVERRIDE || homedir()`). */ -export function factoryConfigHome(env: NodeJS.ProcessEnv = process.env): string { - const configured = trimmed(env.FACTORY_HOME_OVERRIDE); - return path.join(configured ? path.resolve(configured) : home(env), ".factory"); +export function factoryConfigHome(args: HomeArg = {}): string { + const configured = trimmed((args.env ?? process.env).FACTORY_HOME_OVERRIDE); + return path.join(configured ? path.resolve(configured) : baseHome(args), ".factory"); } From e82c8c92e875d083cf9adb776862e7aee65feffb Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:15:29 -0400 Subject: [PATCH 06/18] codex: keep reasoning effort on the thread, not on the app-server process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADE pushed `-c model_reasoning_effort="..."` onto the app-server spawn args. Two problems beyond the obvious one: - `-c` is the highest config layer. Per the documented precedence it outranks the user's ~/.codex/config.toml AND their per-project .codex/config.toml. - It is a process argument, so one chat's selection applied to every thread on that app-server. It also defeated ADE's own per-thread overlay, which was already written correctly — codexThreadConfigArgs omits model_reasoning_effort when nothing is set. Dropping the spawn flag makes the composer's effort selector mean what it says: this chat, this thread. The resolved value is still computed for display. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../main/services/chat/agentChatService.test.ts | 14 ++++++++------ .../src/main/services/chat/agentChatService.ts | 11 ++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 103e20576..7330b2c0d 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -7741,7 +7741,7 @@ describe("createAgentChatService", () => { } }); - it("passes the selected Codex reasoning effort into app-server config", async () => { + it("passes the selected Codex reasoning effort per thread, not on the process", async () => { const laneRootPath = path.join(tmpRoot, "lane-2"); fs.mkdirSync(laneRootPath, { recursive: true }); @@ -7762,11 +7762,13 @@ describe("createAgentChatService", () => { expect(mockState.codexRequestPayloads.some((payload) => payload.method === "thread/start")).toBe(true); }); - expect(spawn).toHaveBeenCalledWith( - "codex", - ["app-server", "-c", "model_reasoning_effort=\"low\""], - expect.any(Object), - ); + // Reasoning effort is a per-chat choice, so it must ride the thread and + // never the process: `-c` is the highest config layer (above the user's + // ~/.codex/config.toml and their per-project .codex/config.toml) and a + // spawn arg would apply to every thread on this app-server. + expect(spawn).toHaveBeenCalledWith("codex", ["app-server"], expect.any(Object)); + const spawnArgs = vi.mocked(spawn).mock.calls[0]?.[1] as string[] | undefined; + expect(spawnArgs?.join(" ")).not.toContain("model_reasoning_effort"); const startPayload = mockState.codexRequestPayloads.find((payload) => payload.method === "thread/start"); const startParams = startPayload?.params as { diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 66e2950a6..c44061908 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -28289,13 +28289,18 @@ export function createAgentChatService(args: { const appServerArgs = ["app-server"]; if (sessionSupportsReasoning(managed.session)) { const descriptor = resolveSessionModelDescriptor(managed.session); - const reasoningEffort = resolveCodexReasoningEffortForRuntime( + // Resolve for display only. Reasoning effort is a per-chat choice, so it + // travels with the thread (codexThreadConfigArgs), never on the process. + // A `-c model_reasoning_effort=...` spawn flag applied to every thread on + // this app-server, not just this chat, and `-c` is the highest config + // layer — above the user's ~/.codex/config.toml AND their per-project + // .codex/config.toml. It also defeated the per-thread overlay, which + // already omits the key correctly when nothing is chosen. + managed.session.reasoningEffort = resolveCodexReasoningEffortForRuntime( managed.session.reasoningEffort, null, descriptor, ); - managed.session.reasoningEffort = reasoningEffort; - appServerArgs.push("-c", `model_reasoning_effort="${reasoningEffort}"`); } const invocation = resolveCliSpawnInvocation(codexExecutable, appServerArgs); const proc = spawn(invocation.command, invocation.args, { From 5138ce5bcd71c3f0eaecfd568bbc47cae2b471a5 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:26:23 -0400 Subject: [PATCH 07/18] opencode: close the plan-mode write hole and stop overriding user config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PLAN MODE COULD WRITE FILES. ade-plan set edit:"deny" but deliberately left the native `task` tool enabled so child sessions would appear in the subagents pane. A spawned subagent runs under its OWN ruleset — OpenCode's `general` is merge(base, {todowrite:"deny"}), and the base is {"*":"allow"}, so edit is ALLOWED there. Plan blocked the direct write and permitted the indirect one. Plan now denies `task`. Also, since OPENCODE_CONFIG_CONTENT is merged LAST and per key — only managed/MDM config outranks it — every key ADE names beats the user's own opencode.json: - share and snapshot are no longer sent. Neither has ADE UI, and snapshot's documented default is true: forcing false silently disabled OpenCode's own /undo and /revert, which restore uncommitted in-turn state that git lanes do not cover. - autoupdate moves to OPENCODE_DISABLE_AUTOUPDATE in the server env. ADE does pin the binary, but that does not need the top-precedence config slot. - provider.ollama / provider.lmstudio were emitted for every session with ADE's default baseURL even when the user had never configured them, deep-merging over the endpoint in their own opencode.json and repointing a configured remote host back at localhost. They are now emitted only when the user typed an endpoint or ADE discovered models. lmstudio is in OpenCode's provider catalog with its own npm and baseURL, so only ollama states npm. Two faithfulness fixes while here: - ade-full-auto now states read:"allow". The base ruleset asks before reading *.env, so "full access" still prompted. external_directory stays "ask": that boundary is ADE's lane worktree, not a permission tier the user chose. - ade-* agents are hidden. Without a mode they defaulted to "all" and appeared in the user's Tab-cycle and @-autocomplete. Deprecated spellings replaced: the ade-plan `tools` map becomes explicit permission entries (OpenCode desugars it to exactly those, and an explicit permission block wins), and maxSteps becomes steps. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/agentChatService.test.ts | 4 +- .../services/opencode/openCodeRuntime.test.ts | 54 ++++++++++++- .../main/services/opencode/openCodeRuntime.ts | 81 +++++++++++-------- .../opencode/openCodeServerManager.ts | 4 + 4 files changed, 107 insertions(+), 36 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 7330b2c0d..746cffc84 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -7863,7 +7863,7 @@ describe("createAgentChatService", () => { expect(getAdeCliAgentEnv).toHaveBeenCalled(); expect(spawn).toHaveBeenCalledWith( "codex", - ["app-server", "-c", "model_reasoning_effort=\"medium\""], + ["app-server"], expect.objectContaining({ env: expect.objectContaining({ PATH: "/tmp/ade-cli/bin", @@ -20275,7 +20275,7 @@ describe("createAgentChatService", () => { expect(spawn).toHaveBeenCalledWith( "codex", - ["app-server", "-c", "model_reasoning_effort=\"medium\""], + ["app-server"], expect.objectContaining({ detached: process.platform !== "win32" }), ); expect(processKillSpy).toHaveBeenCalledWith(-99999, "SIGTERM"); diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts index 1b5588a44..51e854265 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts @@ -149,8 +149,17 @@ describe("openCodeRuntime", () => { config: expect.objectContaining({ agent: expect.objectContaining({ "ade-plan": expect.objectContaining({ - tools: expect.objectContaining({ code_search: false, web_search: false }), - permission: expect.objectContaining({ question: "allow" }), + // The deprecated `tools` map is gone; OpenCode desugars it into + // these same permission keys and an explicit permission block wins. + permission: expect.objectContaining({ + question: "allow", + websearch: "deny", + skill: "deny", + // Plan denies edit, so it must deny task: a spawned subagent runs + // under its own ruleset with edit allowed, which let plan mode + // write files through a child session. + task: "deny", + }), }), "ade-helper": expect.objectContaining({ permission: expect.objectContaining({ question: "deny" }), @@ -360,3 +369,44 @@ describe("refreshOpenCodeSessionToolSelection", () => { } }); }); + +describe("buildOpenCodeConfig user-owned keys", () => { + const config = (): Record => + buildOpenCodeConfig({ projectConfig: { ai: {} } as any }) as Record; + + it("does not force share or snapshot over the user's opencode.json", () => { + // OPENCODE_CONFIG_CONTENT merges last, so naming these would beat the user's + // own file. snapshot's documented default is true, and forcing false + // silently disables OpenCode's /undo and /revert. + expect(config()).not.toHaveProperty("share"); + expect(config()).not.toHaveProperty("snapshot"); + expect(config()).not.toHaveProperty("autoupdate"); + }); + + it("denies task in plan mode so a subagent cannot write for it", () => { + // The regression: plan denied `edit` but left `task` open, and a spawned + // subagent runs under its own ruleset where edit is allowed. + const plan = config().agent["ade-plan"].permission; + expect(plan.edit).toBe("deny"); + expect(plan.task).toBe("deny"); + }); + + it("lets full access read without prompting", () => { + // OpenCode's base ruleset asks before reading *.env, so full access + // prompted until read was stated. + expect(config().agent["ade-full-auto"].permission.read).toBe("allow"); + }); + + it("keeps ADE's own modes out of the user's agent picker", () => { + for (const name of ["ade-plan", "ade-edit", "ade-full-auto", "ade-helper"]) { + expect(config().agent[name].hidden).toBe(true); + } + }); + + it("omits local providers the user never configured", () => { + // An ADE-invented baseURL merges over the endpoint in the user's own + // opencode.json, repointing a configured remote host back at localhost. + expect(config().provider ?? {}).not.toHaveProperty("ollama"); + expect(config().provider ?? {}).not.toHaveProperty("lmstudio"); + }); +}); diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts index ed1cf0a24..c683d63b4 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts @@ -43,20 +43,6 @@ import { export type OpenCodeAgentProfile = "ade-plan" | "ade-edit" | "ade-full-auto" | "ade-helper"; -const ADE_PLAN_TOOL_SELECTION: Record = { - // ADE planning uses coordinator tools such as spawn_worker. The native - // `task` subagent tool is intentionally allowed so OpenCode child sessions - // surface in the desktop / TUI subagents panes. - codesearch: false, - code_search: false, - filesearch: false, - file_search: false, - websearch: false, - web_search: false, - skill: false, - skills: false, -}; - export type OpenCodeSessionHandle = { client: OpencodeClient; v2Client: OpenCodeV2Client; @@ -196,22 +182,26 @@ export function resolveOpenCodeModelSelection(descriptor: ModelDescriptor): { }; } +type OpenCodePermissionAction = "allow" | "ask" | "deny"; + function buildPermissionConfig( permissionMode: PermissionMode, -): { - edit: "allow" | "ask" | "deny"; - bash: "allow" | "ask" | "deny"; - webfetch: "allow" | "ask" | "deny"; - doom_loop: "allow" | "ask" | "deny"; - external_directory: "allow" | "ask" | "deny"; - question: "allow" | "ask" | "deny"; -} { +): Record> { if (permissionMode === "full-auto") { return { edit: "allow", bash: "allow", webfetch: "allow", doom_loop: "allow", + // Every key OpenCode does not gate resolves to "allow" from its base "*" + // rule, but `read` is not one of them: the base ruleset asks before + // reading *.env / *.env.*, so full access still prompted. Full access + // means no prompts. + read: "allow", + task: "allow", + // external_directory stays "ask". That boundary is ADE's lane worktree, + // not a permission tier the user picked — the same reason the system + // prompt confines edits to the lane. external_directory: "ask", question: "allow", }; @@ -223,8 +213,18 @@ function buildPermissionConfig( bash: "ask", webfetch: "allow", doom_loop: "ask", + // Plan denies `edit`, so it must deny `task` too. A spawned subagent runs + // under its own ruleset — OpenCode's `general` is merge(base, todowrite + // deny), i.e. edit ALLOWED — so leaving `task` open let plan mode write + // files indirectly through a child session. Plan has to mean plan. + task: "deny", external_directory: "deny", question: "allow", + // Replaces the deprecated agent-level `tools` map. OpenCode desugars that + // map into exactly these permission entries, and an explicit `permission` + // block wins over it, so stating them directly is the supported spelling. + websearch: "deny", + skill: "deny", }; } @@ -367,12 +367,18 @@ function buildProviderConfig( models[modelId] = { name: modelId }; } } - const rawEndpoint = trimToUndefined(settings?.endpoint) ?? getLocalProviderDefaultEndpoint(family); + const endpoint = trimToUndefined(settings?.endpoint); + // Say nothing about a provider the user never set up. This config is merged + // last and per key, so an ADE-invented baseURL would overwrite the endpoint + // in the user's own opencode.json — repointing a configured remote host back + // at localhost. Only an endpoint the user actually typed, or models ADE + // discovered, justify naming the provider at all. + if (!endpoint && !Object.keys(models).length) return; provider[family] = { - npm: "@ai-sdk/openai-compatible", - options: { - baseURL: ensureOpenCodeBaseURL(rawEndpoint), - }, + // lmstudio ships in OpenCode's provider catalog with its own npm package + // and baseURL; ollama does not, so only ollama needs one stated here. + ...(family === "ollama" ? { npm: "@ai-sdk/openai-compatible" } : {}), + ...(endpoint ? { options: { baseURL: ensureOpenCodeBaseURL(endpoint) } } : {}), ...(Object.keys(models).length > 0 ? { models } : {}), }; }; @@ -483,26 +489,37 @@ export function buildOpenCodeConfig(args: BuildOpenCodeConfigArgs): OpenCodeConf question: "deny", } as const; + // OPENCODE_CONFIG_CONTENT is merged last, so anything named here outranks the + // user's opencode.json and only managed/MDM config beats it. `share` and + // `snapshot` are therefore omitted: neither has ADE UI, and forcing + // snapshot:false silently disabled OpenCode's own /undo and /revert, whose + // documented default is true. `autoupdate` moved to OPENCODE_DISABLE_AUTOUPDATE + // in the server env — ADE does pin the binary, but that does not need the + // highest-precedence config slot. return { - share: "disabled", - autoupdate: false, - snapshot: false, ...(provider ? { provider } : {}), ...(args.mcp ? { mcp: args.mcp } : {}), agent: { + // hidden: these are ADE's own modes, not agents the user should see in + // their Tab-cycle or @-autocomplete. Without a `mode` they would default + // to "all" and show up in the picker. "ade-plan": { permission: buildPermissionConfig("plan"), - tools: ADE_PLAN_TOOL_SELECTION, + hidden: true, }, "ade-edit": { permission: buildPermissionConfig("edit"), + hidden: true, }, "ade-full-auto": { permission: buildPermissionConfig("full-auto"), + hidden: true, }, "ade-helper": { permission: helperPermission, - maxSteps: 1, + // `steps`; `maxSteps` is the deprecated spelling. + steps: 1, + hidden: true, }, }, }; diff --git a/apps/desktop/src/main/services/opencode/openCodeServerManager.ts b/apps/desktop/src/main/services/opencode/openCodeServerManager.ts index 910a999fb..a18f4178f 100644 --- a/apps/desktop/src/main/services/opencode/openCodeServerManager.ts +++ b/apps/desktop/src/main/services/opencode/openCodeServerManager.ts @@ -798,6 +798,10 @@ function mergeOpenCodeConfig( function buildUserOpenCodeEnv(config: OpenCodeConfig): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = { ...process.env }; + // ADE resolves and pins the OpenCode binary, so its updater must stay off. + // OpenCode exposes a dedicated env var for exactly this, which keeps the + // highest-precedence config slot free of a key the user might own. + env.OPENCODE_DISABLE_AUTOUPDATE = "1"; const inheritedContent = env.OPENCODE_CONFIG_CONTENT?.trim(); if (inheritedContent) { try { From 7659e28bebc2001517dbc04ace03d6b22064733b Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:24:27 -0400 Subject: [PATCH 08/18] quality: fix the passthrough that was undone downstream, plus review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real defects found by the correctness track, and the first one silently defeated half of the Droid change: - droidSdkWorker rebuilt interactionMode unconditionally. buildDroidSdkSessionSettings correctly omitted it for a session with no chosen mode, and the worker then materialised DroidInteractionMode.Auto and sent it on createSession and every updateSettings — restating at the highest tier exactly what the omission existed to leave alone. toDroidInteractionMode now maps "auto" explicitly and returns undefined otherwise; both call sites spread conditionally. - Isolated (orchestration-lead) OpenCode servers lost autoupdate suppression: buildIsolatedOpenCodeEnv strips every OPENCODE_* var and rebuilds from scratch, so it never saw the var set in buildUserOpenCodeEnv, and a lead's server would self-update the binary ADE pins. - CLAUDE_CONFIG_DIR was outranked by the ancestor walk. A lane normally sits under $HOME, so the walk reached the real ~/.claude and ranked it as a project tier ABOVE the relocated user tier — the normal case, not an edge case. The stale home settings won and ADE passed them at flag tier, the exact class this branch exists to remove. Regression test included; it reports StaleHomeStyle without the fix. discoverClaudePlugins was reading the plugin registry from the same wrong directory. Maintainability findings applied: - Deleted the sandboxEnabled boolean. It survived only to keep one call site compiling, and that call site — providerTaskRunner — still emitted the explicit `false` this branch removed from the worker, so the user's ~/.cursor/sandbox.json was still being suppressed there. One field, one encoding, and the compiler found the straggler. - Deleted resolveSessionDroidPermissionMode (one caller, applying a fallback its own caller had already applied) and the unreachable default branch that hid exhaustiveness from the compiler. - buildDroidSdkSessionSettings computes one `stated` object instead of three overlapping booleans, so spec-mode fields cannot be emitted without the mode that justifies them. - Removed the codex reasoning-effort spawn block: after the flag was dropped it only recomputed a value thread/start overwrites moments later. - claudeOutputStyles uses the shared claudeConfigHome rather than the duplicate resolver this branch had added a few files away. - Narrowed types that carried members which can no longer occur, and gave buildPermissionConfig a keyed union — the OpenCode SDK absorbs unknown permission keys through an index signature, so a typo would have compiled and silently failed to apply. - Collapsed the rationale that had been restated in five adapters into one doc block in providerConfigHomes, and added the test that module never had. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/ai/providerTaskRunner.test.ts | 4 +- .../main/services/ai/providerTaskRunner.ts | 6 +- .../main/services/chat/agentChatService.ts | 61 +++++---------- .../services/chat/claudeOutputStyles.test.ts | 39 ++++++++++ .../main/services/chat/claudeOutputStyles.ts | 36 ++++----- .../services/chat/cursorSdkPolicy.test.ts | 14 ++-- .../src/main/services/chat/cursorSdkPolicy.ts | 10 +-- .../src/main/services/chat/cursorSdkWorker.ts | 11 +-- .../services/chat/droidModelsDiscovery.ts | 2 +- .../main/services/chat/droidSdkProtocol.ts | 6 +- .../src/main/services/chat/droidSdkWorker.ts | 27 ++++--- .../main/services/opencode/openCodeRuntime.ts | 25 ++++++- .../opencode/openCodeServerManager.ts | 6 +- .../shared/providerConfigHomes.test.ts | 74 +++++++++++++++++++ .../services/shared/providerConfigHomes.ts | 24 +++++- 15 files changed, 242 insertions(+), 103 deletions(-) create mode 100644 apps/desktop/src/main/services/shared/providerConfigHomes.test.ts diff --git a/apps/desktop/src/main/services/ai/providerTaskRunner.test.ts b/apps/desktop/src/main/services/ai/providerTaskRunner.test.ts index 7f6e6062a..0c1ecaf70 100644 --- a/apps/desktop/src/main/services/ai/providerTaskRunner.test.ts +++ b/apps/desktop/src/main/services/ai/providerTaskRunner.test.ts @@ -315,7 +315,9 @@ describe("runProviderTask", () => { const createOptions = cursorAgentCreateMock.mock.calls[0]![0] as Record; expect(createOptions.mode).toBe("agent"); expect(createOptions.local.autoReview).toBe(true); - expect(createOptions.local.sandboxOptions).toEqual({ enabled: false }); + // Middle-trust maps to Cursor "agent", where ADE has no sandbox opinion: an + // explicit false would make the SDK skip the user's ~/.cursor/sandbox.json. + expect(createOptions.local.sandboxOptions).toBeUndefined(); expect(createOptions.tools).toBeUndefined(); }); diff --git a/apps/desktop/src/main/services/ai/providerTaskRunner.ts b/apps/desktop/src/main/services/ai/providerTaskRunner.ts index cc9b43097..152c70f0f 100644 --- a/apps/desktop/src/main/services/ai/providerTaskRunner.ts +++ b/apps/desktop/src/main/services/ai/providerTaskRunner.ts @@ -388,7 +388,11 @@ async function runCursorTask(args: ProviderTaskRunnerArgs): Promise, - fallback: AgentChatDroidPermissionMode, -): AgentChatDroidPermissionMode { - return resolveSessionDroidPermissionModeOrNull(session) ?? fallback; -} - function applyLocalHarnessPermissionMode(args: { descriptor?: ModelDescriptor; requestedPermissionMode?: AgentChatSession["permissionMode"]; @@ -7323,9 +7316,8 @@ function resolveDroidRuntimeModelId( } function resolveDroidSdkAutonomyLevel( - session: Pick, + mode: AgentChatDroidPermissionMode, ): DroidSdkSessionSettings["autonomyLevel"] { - const mode = resolveSessionDroidPermissionMode(session, "auto-low"); switch (mode) { case "read-only": return "off"; @@ -7339,8 +7331,6 @@ function resolveDroidSdkAutonomyLevel( return "medium"; case "auto-high": return "high"; - default: - return "low"; } } @@ -28286,22 +28276,10 @@ export function createAgentChatService(args: { }); throw error; } + // Reasoning effort travels with the thread (codexThreadConfigArgs), never on + // the process: `-c` outranks the user's config.toml and would apply to every + // thread on this app-server, not just this chat. const appServerArgs = ["app-server"]; - if (sessionSupportsReasoning(managed.session)) { - const descriptor = resolveSessionModelDescriptor(managed.session); - // Resolve for display only. Reasoning effort is a per-chat choice, so it - // travels with the thread (codexThreadConfigArgs), never on the process. - // A `-c model_reasoning_effort=...` spawn flag applied to every thread on - // this app-server, not just this chat, and `-c` is the highest config - // layer — above the user's ~/.codex/config.toml AND their per-project - // .codex/config.toml. It also defeated the per-thread overlay, which - // already omits the key correctly when nothing is chosen. - managed.session.reasoningEffort = resolveCodexReasoningEffortForRuntime( - managed.session.reasoningEffort, - null, - descriptor, - ); - } const invocation = resolveCliSpawnInvocation(codexExecutable, appServerArgs); const proc = spawn(invocation.command, invocation.args, { cwd: managed.laneWorktreePath, @@ -35127,31 +35105,26 @@ export function createAgentChatService(args: { const chosenMode = resolveSessionDroidPermissionModeOrNull(managed.session); const planRequested = managed.session.interactionMode === "plan" || managed.session.permissionMode === "plan"; - // Say nothing when the user picked nothing, so Droid resolves autonomy from - // their own settings.json exactly as the terminal path already does — - // droidSettingsJson omits sessionDefaultSettings when permissionMode is null. - const statesAutonomy = chosenMode !== null - || planRequested - || isOrchestrationLeadSession(managed.session); - const interactionMode: DroidSdkSessionSettings["interactionMode"] = - chosenMode === "agi" - ? "agi" - : resolveDroidSdkInteractionMode(managed.session); + // Mirrors the terminal path: droidSettingsJson omits sessionDefaultSettings + // when permissionMode is null, letting the user's settings.json decide. + const stated = chosenMode !== null || planRequested || isOrchestrationLeadSession(managed.session) + ? { + autonomyLevel: resolveDroidSdkAutonomyLevel(chosenMode ?? "auto-low"), + interactionMode: chosenMode === "agi" + ? "agi" as const + : resolveDroidSdkInteractionMode(managed.session), + } + : null; return { modelId, - ...(statesAutonomy - ? { - autonomyLevel: resolveDroidSdkAutonomyLevel(managed.session), - interactionMode, - } - : {}), + ...(stated ?? {}), // Droid's own editor/terminal tools live outside ADE's toolset, so a lead // has to have them withheld natively as well. ...(isOrchestrationLeadSession(managed.session) ? { disabledToolCategories: ORCHESTRATION_LEAD_DENIED_DROID_TOOL_CATEGORIES } : {}), ...(reasoningEffort ? { reasoningEffort } : {}), - ...(interactionMode === "spec" + ...(stated?.interactionMode === "spec" ? { specModeModelId: modelId, ...(reasoningEffort ? { specModeReasoningEffort: reasoningEffort } : {}), diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts index 664d3f34b..4e5f54ef2 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.test.ts @@ -282,3 +282,42 @@ describe("settings precedence", () => { expect(readClaudeWorkflowSizeGuideline(tmpRoot)).toBe("large"); }); }); + +describe("CLAUDE_CONFIG_DIR vs the ancestor walk", () => { + it("does not let a stale real ~/.claude outrank the relocated config dir", () => { + // The regression: a lane normally sits UNDER $HOME, so the ancestor walk + // reaches ~/.claude and ranked it as a project tier above the user tier. + // With CLAUDE_CONFIG_DIR pointing elsewhere the stale home settings won, + // and ADE then passed that at flag tier — overriding the very directory the + // CLI reads. The other tests cannot catch this: their cwd is a SIBLING of + // the fake home, so the walk never reaches it. + const laneRoot = path.join(homeRoot, "proj", "lane"); + fs.mkdirSync(path.join(laneRoot, ".claude"), { recursive: true }); + + const relocated = path.join(homeRoot, "relocated-claude"); + fs.mkdirSync(relocated, { recursive: true }); + fs.writeFileSync(path.join(relocated, "settings.json"), JSON.stringify({ outputStyle: "RelocatedStyle" })); + + fs.mkdirSync(path.join(homeRoot, ".claude"), { recursive: true }); + fs.writeFileSync( + path.join(homeRoot, ".claude", "settings.json"), + JSON.stringify({ outputStyle: "StaleHomeStyle" }), + ); + + process.env.CLAUDE_CONFIG_DIR = relocated; + expect(readClaudeOutputStyleSelection(laneRoot)).toBe("RelocatedStyle"); + }); + + it("still reads the real home when no override is set", () => { + const laneRoot = path.join(homeRoot, "proj", "lane"); + fs.mkdirSync(laneRoot, { recursive: true }); + fs.mkdirSync(path.join(homeRoot, ".claude"), { recursive: true }); + fs.writeFileSync( + path.join(homeRoot, ".claude", "settings.json"), + JSON.stringify({ outputStyle: "HomeStyle" }), + ); + + delete process.env.CLAUDE_CONFIG_DIR; + expect(readClaudeOutputStyleSelection(laneRoot)).toBe("HomeStyle"); + }); +}); diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts index c63cf2b70..175d46063 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { parse as parseYaml } from "yaml"; import type { AgentChatClaudeOutputStyle, AgentChatClaudePlugin } from "../../../shared/types/chat"; +import { claudeConfigHome } from "../shared/providerConfigHomes"; import { writeTextAtomic } from "../shared/utils"; const MAX_ANCESTOR_DEPTH = 25; @@ -45,6 +46,7 @@ type ClaudePluginManifest = { type ClaudeSettingsLocal = Record & { outputStyle?: unknown; + workflowSizeGuideline?: unknown; enabledPlugins?: unknown; }; @@ -158,7 +160,7 @@ function pathsOverlap(left: string, right: string): boolean { } function installedClaudePluginPaths(cwd: string, enabledKeys: Set, enabledNames: Set): string[] { - const registry = readJsonObject(path.join(os.homedir(), ".claude", "plugins", "installed_plugins.json")); + const registry = readJsonObject(path.join(claudeConfigHome({ homeDir: os.homedir() }), "plugins", "installed_plugins.json")); const plugins = isRecord(registry?.plugins) ? registry.plugins : null; if (!plugins) return []; @@ -208,16 +210,6 @@ function ancestorClaudeRoots(cwd: string): string[] { return roots; } -/** - * The user-level Claude root. `CLAUDE_CONFIG_DIR` relocates it wholesale — the - * CLI reads settings, styles and plugins from there instead of `~/.claude`, and - * every other ADE module that touches Claude config already honours it. - */ -function userClaudeRoot(): string { - const configured = process.env.CLAUDE_CONFIG_DIR?.trim(); - return configured?.length ? path.resolve(configured) : path.join(path.resolve(os.homedir()), ".claude"); -} - function claudeRootsByPrecedence(cwd: string): string[] { const roots: string[] = []; const seen = new Set(); @@ -227,8 +219,19 @@ function claudeRootsByPrecedence(cwd: string): string[] { roots.push(root); }; - for (const root of ancestorClaudeRoots(cwd)) addRoot(root); - addRoot(userClaudeRoot()); + const userRoot = claudeConfigHome({ homeDir: os.homedir() }); + const realHomeRoot = path.join(path.resolve(os.homedir()), ".claude"); + // A lane normally sits under $HOME, so the ancestor walk reaches ~/.claude and + // would rank it as a project tier ABOVE the user tier. That is wrong whenever + // CLAUDE_CONFIG_DIR moved the user tier elsewhere: the stale real ~/.claude + // would outrank the directory the CLI actually reads. + const skipRealHomeRoot = userRoot !== realHomeRoot; + + for (const root of ancestorClaudeRoots(cwd)) { + if (skipRealHomeRoot && root === realHomeRoot) continue; + addRoot(root); + } + addRoot(userRoot); return roots; } @@ -352,7 +355,7 @@ export function discoverClaudeOutputStyles(cwd: string): AgentChatClaudeOutputSt for (const style of CLAUDE_BUILT_IN_OUTPUT_STYLES) add(style); const roots = claudeRootsByPrecedence(cwd); - const homeClaudeRoot = userClaudeRoot(); + const homeClaudeRoot = claudeConfigHome({ homeDir: os.homedir() }); const cwdClaudeRoot = path.resolve(cwd, ".claude"); for (const root of roots) { const resolvedRoot = path.resolve(root); @@ -401,10 +404,7 @@ export function readClaudeSettingsLocal(cwd: string): ClaudeSettingsLocal { * lane `settings.local.json`, lane `settings.json`, each ancestor root, then * `~/.claude`. Returns null when no file declares the key. * - * ADE reads these only to decide whether it has anything to say. A key nobody - * declares must stay absent from the SDK options — ADE passes its settings at - * flag tier, which outranks every file, so substituting a default here silently - * overrides the user's global configuration. + * See providerConfigHomes.ts for why absence must stay absent. */ function readClaudeSettingsValue(cwd: string, key: string): string | null { for (const root of claudeRootsByPrecedence(cwd)) { diff --git a/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts b/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts index 3d9497091..2421ca162 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkPolicy.test.ts @@ -58,13 +58,13 @@ describe("Cursor SDK policy", () => { const expected: Record = { - agent: { mode: "agent", sandboxEnabled: false, autoReview: true }, - ask: { mode: "plan", tools: ["read", "grep", "glob", "ls"], sandboxEnabled: true, autoReview: false }, - plan: { mode: "plan", tools: ["read", "grep", "glob", "ls"], sandboxEnabled: true, autoReview: false }, - "full-auto": { mode: "agent", sandboxEnabled: false, autoReview: false }, + agent: { mode: "agent", sandboxDirective: "inherit", autoReview: true }, + ask: { mode: "plan", tools: ["read", "grep", "glob", "ls"], sandboxDirective: "enable", autoReview: false }, + plan: { mode: "plan", tools: ["read", "grep", "glob", "ls"], sandboxDirective: "enable", autoReview: false }, + "full-auto": { mode: "agent", sandboxDirective: "disable", autoReview: false }, }; for (const modeId of ["agent", "ask", "plan", "full-auto"] as const) { const policy = resolveCursorSdkPolicy({ cursorModeId: modeId }); @@ -74,7 +74,7 @@ describe("Cursor SDK policy", () => { expect(local.mode).toBe(expected[modeId]!.mode); expect(local.mode).not.toBe("auto"); expect(local.autoReview).toBe(expected[modeId]!.autoReview); - expect(local.sandboxEnabled).toBe(expected[modeId]!.sandboxEnabled); + expect(local.sandboxDirective).toBe(expected[modeId]!.sandboxDirective); if (expected[modeId]!.tools) { expect(local.tools).toEqual(expected[modeId]!.tools); } else { @@ -100,7 +100,7 @@ describe("Cursor SDK policy", () => { mode: "plan", tools: ["read", "grep", "glob", "ls"], autoReview: false, - sandboxEnabled: false, + sandboxDirective: "disable", }); }); diff --git a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts index 2e43e8412..61581e949 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts @@ -67,11 +67,8 @@ export type CursorSdkReadonlyTool = (typeof CURSOR_SDK_READONLY_TOOLS)[number]; * returns `insecure_none` without ever reading the user's ~/.cursor/sandbox.json, * while absent lets that file decide. So ADE needs three states, not two. * - * - "enable" — ADE asks for a sandbox (ask/plan). A user policy still wins. - * - "disable" — full access means no sandbox, even for a user who wrote a policy. - * Also used after a ConfigurationError, where the environment - * cannot sandbox and the alternative is a hard failure. - * - "inherit" — ADE has no opinion. The user's sandbox.json decides. + * "disable" also covers the retry after a ConfigurationError, where the + * environment cannot sandbox at all and the alternative is a hard failure. */ export type CursorSdkSandboxDirective = "enable" | "disable" | "inherit"; @@ -80,7 +77,6 @@ export type CursorSdkLocalRunOptions = { tools?: string[]; disallowedTools?: string[]; autoReview: boolean; - sandboxEnabled: boolean; sandboxDirective: CursorSdkSandboxDirective; }; @@ -103,7 +99,6 @@ export function buildCursorSdkLocalRunOptions( args?: { sandboxSupported?: boolean }, ): CursorSdkLocalRunOptions { const sandboxSupported = args?.sandboxSupported !== false; - const sandboxEnabled = policy.sandbox === "cursor-native" && sandboxSupported; const sandboxDirective: CursorSdkSandboxDirective = !sandboxSupported ? "disable" : policy.sandbox === "cursor-native" @@ -116,7 +111,6 @@ export function buildCursorSdkLocalRunOptions( ...(policy.tools?.length ? { tools: [...policy.tools] } : {}), ...(policy.disallowedTools?.length ? { disallowedTools: [...policy.disallowedTools] } : {}), autoReview: policy.autoReview, - sandboxEnabled, sandboxDirective, }; } diff --git a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts index dc7ca9787..8233c1992 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts @@ -245,10 +245,7 @@ function buildLocalAgentOptions(init: CursorSdkWorkerInit): AgentOptionsWithAdeM local: { cwd: init.laneRoot, settingSources: cursorSdkSettingSources(init.policy), - // `false` and absent are not the same thing here. An explicit `false` - // returns `insecure_none` without ever reading the user's - // ~/.cursor/sandbox.json, so it is a deliberate statement rather than a - // neutral default. See CursorSdkSandboxDirective for the three cases. + // See CursorSdkSandboxDirective: absent is a third state, not a falsy off. ...(local.sandboxDirective === "inherit" ? {} : { sandboxOptions: { enabled: local.sandboxDirective === "enable" } }), @@ -515,7 +512,11 @@ async function initWorker(init: CursorSdkWorkerInit): Promise<{ agentId: string; useHttp1ForAgent, mode: agentOptions.mode ?? null, autoReview: agentOptions.local?.autoReview === true, - sandboxEnabled: agentOptions.local?.sandboxOptions?.enabled === true, + // Absent is a third state, not a falsy "off" — logging a boolean here + // collapsed "disable" and "inherit" into the same line. + sandboxDirective: agentOptions.local?.sandboxOptions === undefined + ? "inherit" + : agentOptions.local.sandboxOptions.enabled ? "enable" : "disable", tools: agentOptions.tools ?? null, disallowedTools: agentOptions.disallowedTools ?? null, }, diff --git a/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts b/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts index 08dee172b..b99d57475 100644 --- a/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts +++ b/apps/desktop/src/main/services/chat/droidModelsDiscovery.ts @@ -1,6 +1,6 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; -import { homedir, tmpdir } from "node:os"; +import { tmpdir } from "node:os"; import { createDynamicDroidCliModelDescriptor, sortDroidCliDescriptorsForPicker, diff --git a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts index 9d3dc686e..e5f0b49ce 100644 --- a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts +++ b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts @@ -29,9 +29,9 @@ export type DroidSdkSessionSettings = { */ autonomyLevel?: DroidSdkAutonomyLevel; interactionMode?: DroidSdkInteractionMode; - reasoningEffort?: DroidSdkReasoningEffort | null; - specModeModelId?: string | null; - specModeReasoningEffort?: DroidSdkReasoningEffort | null; + reasoningEffort?: DroidSdkReasoningEffort; + specModeModelId?: string; + specModeReasoningEffort?: DroidSdkReasoningEffort; /** * Droid tool categories to withhold from the model, resolved to concrete * `disabledToolIds` in the worker (tool ids are build-specific, categories diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index e3c12e341..aec43cafa 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -53,17 +53,24 @@ function coerceReasoning(value: DroidSdkReasoningEffort | null | undefined): Dro return value?.trim() ? value as DroidSdkTypes.ReasoningEffort : undefined; } +/** + * Undefined in, undefined out. An omitted interactionMode means "ADE has no + * opinion, let ~/.factory/settings.json decide" — materialising a default here + * would restate it at the highest precedence and undo the omission upstream. + */ function toDroidInteractionMode( sdk: DroidSdkModule, mode: DroidSdkSessionSettings["interactionMode"], -): DroidSdkTypes.DroidInteractionMode { +): DroidSdkTypes.DroidInteractionMode | undefined { switch (mode) { case "spec": return sdk.DroidInteractionMode.Spec; case "agi": return sdk.DroidInteractionMode.AGI; - default: + case "auto": return sdk.DroidInteractionMode.Auto; + default: + return undefined; } } @@ -72,12 +79,15 @@ function sessionOptions( init: DroidSdkWorkerInit, settings: DroidSdkSessionSettings, ): DroidSdkTypes.CreateSessionOptions { + const interactionMode = toDroidInteractionMode(sdk, settings.interactionMode); return { cwd: init.laneRoot, execPath: init.droidPath, modelId: settings.modelId, - autonomyLevel: settings.autonomyLevel as DroidSdkTypes.AutonomyLevel, - interactionMode: toDroidInteractionMode(sdk, settings.interactionMode), + // Omitted, not defaulted: both keys are optional in the SDK and each + // resolves independently from the user's settings.json when absent. + ...(settings.autonomyLevel ? { autonomyLevel: settings.autonomyLevel as DroidSdkTypes.AutonomyLevel } : {}), + ...(interactionMode ? { interactionMode } : {}), reasoningEffort: coerceReasoning(settings.reasoningEffort), specModeModelId: settings.specModeModelId?.trim() || undefined, specModeReasoningEffort: coerceReasoning(settings.specModeReasoningEffort), @@ -239,9 +249,7 @@ function normalizeAvailableModels(initResult: unknown): DroidSdkReady["available * The model Droid actually resolved for this session. * * `initResult.currentModelId` does not exist — @factory/droid-sdk reports the - * resolved settings under `initResult.settings`, so the old read was dead code - * that always produced null, and every caller downstream silently fell back to - * ADE's own value instead of adopting Droid's. + * resolved settings under `initResult.settings`. */ function readResolvedModelId(initResult: unknown): string | null { const record = initResult && typeof initResult === "object" ? initResult as Record : null; @@ -334,10 +342,11 @@ async function applySettings(settings: DroidSdkSessionSettings): Promise { if (disabledToolIds?.length) await session.updateSettings({ disabledToolIds }); return; } + const updateInteractionMode = toDroidInteractionMode(sdk, settings.interactionMode); await session.updateSettings({ modelId: settings.modelId, - autonomyLevel: settings.autonomyLevel as DroidSdkTypes.AutonomyLevel, - interactionMode: toDroidInteractionMode(sdk, settings.interactionMode), + ...(settings.autonomyLevel ? { autonomyLevel: settings.autonomyLevel as DroidSdkTypes.AutonomyLevel } : {}), + ...(updateInteractionMode ? { interactionMode: updateInteractionMode } : {}), reasoningEffort: coerceReasoning(settings.reasoningEffort), ...(disabledToolIds ? { disabledToolIds } : {}), }); diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts index c683d63b4..eed026cee 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts @@ -16,7 +16,6 @@ import { import { decodeOpenCodeRegistryId, ensureOpenCodeBaseURL, - getLocalProviderDefaultEndpoint, type LocalProviderFamily, type ModelDescriptor, } from "../../../shared/modelRegistry"; @@ -184,9 +183,29 @@ export function resolveOpenCodeModelSelection(descriptor: ModelDescriptor): { type OpenCodePermissionAction = "allow" | "ask" | "deny"; +/** + * The permission keys ADE sets. The OpenCode SDK's own type declares only five + * of these and absorbs the rest through an index signature, so a typo would + * compile and silently fail to apply — `websearch` vs `web_search` is a pair + * this codebase has already been bitten by. + */ +type OpenCodePermissionKey = + | "edit" + | "bash" + | "webfetch" + | "doom_loop" + | "external_directory" + | "question" + | "read" + | "task" + | "websearch" + | "skill"; + +type OpenCodePermissionConfig = Partial>; + function buildPermissionConfig( permissionMode: PermissionMode, -): Record> { +): OpenCodePermissionConfig { if (permissionMode === "full-auto") { return { edit: "allow", @@ -487,7 +506,7 @@ export function buildOpenCodeConfig(args: BuildOpenCodeConfigArgs): OpenCodeConf doom_loop: "deny", external_directory: "deny", question: "deny", - } as const; + } as const satisfies OpenCodePermissionConfig; // OPENCODE_CONFIG_CONTENT is merged last, so anything named here outranks the // user's opencode.json and only managed/MDM config beats it. `share` and diff --git a/apps/desktop/src/main/services/opencode/openCodeServerManager.ts b/apps/desktop/src/main/services/opencode/openCodeServerManager.ts index a18f4178f..cb9206f92 100644 --- a/apps/desktop/src/main/services/opencode/openCodeServerManager.ts +++ b/apps/desktop/src/main/services/opencode/openCodeServerManager.ts @@ -773,6 +773,9 @@ function buildIsolatedOpenCodeEnv( OPENCODE_CONFIG_DIR: path.join(paths.configHome, "opencode"), OPENCODE_CONFIG_CONTENT: JSON.stringify(config ?? {}), OPENCODE_DISABLE_PROJECT_CONFIG: "1", + // This builder strips every OPENCODE_* var from process.env, so the + // suppression set in buildUserOpenCodeEnv does not reach an isolated lead. + OPENCODE_DISABLE_AUTOUPDATE: "1", [ADE_OPENCODE_MANAGED_ENV]: "1", [ADE_OPENCODE_OWNER_PID_ENV]: String(process.pid), }; @@ -799,8 +802,7 @@ function mergeOpenCodeConfig( function buildUserOpenCodeEnv(config: OpenCodeConfig): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = { ...process.env }; // ADE resolves and pins the OpenCode binary, so its updater must stay off. - // OpenCode exposes a dedicated env var for exactly this, which keeps the - // highest-precedence config slot free of a key the user might own. + // OpenCode's dedicated env var does this without occupying a config key. env.OPENCODE_DISABLE_AUTOUPDATE = "1"; const inheritedContent = env.OPENCODE_CONFIG_CONTENT?.trim(); if (inheritedContent) { diff --git a/apps/desktop/src/main/services/shared/providerConfigHomes.test.ts b/apps/desktop/src/main/services/shared/providerConfigHomes.test.ts new file mode 100644 index 000000000..3d43ba784 --- /dev/null +++ b/apps/desktop/src/main/services/shared/providerConfigHomes.test.ts @@ -0,0 +1,74 @@ +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const HOME = "/home/tester"; + +// The module imports `{ homedir }` by name, so a spy on the default export +// would not intercept it — that named import is deliberate, because suites that +// mock node:os by spreading the real module leave the default export intact. +vi.mock("node:os", async (importOriginal) => ({ + ...(await importOriginal() as object), + homedir: () => HOME, +})); + +const { claudeConfigHome, codexConfigHome, factoryConfigHome } = await import("./providerConfigHomes"); +const ENV_KEYS = ["CLAUDE_CONFIG_DIR", "CODEX_HOME", "FACTORY_HOME_OVERRIDE"] as const; + +let saved: Partial>; + +beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((key) => [key, process.env[key]])); + for (const key of ENV_KEYS) delete process.env[key]; +}); + +afterEach(() => { + for (const key of ENV_KEYS) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } + vi.restoreAllMocks(); +}); + +describe("providerConfigHomes", () => { + it("falls back to the home directory for every provider", () => { + expect(claudeConfigHome()).toBe(path.join(HOME, ".claude")); + expect(codexConfigHome()).toBe(path.join(HOME, ".codex")); + expect(factoryConfigHome()).toBe(path.join(HOME, ".factory")); + }); + + it("treats CLAUDE_CONFIG_DIR and CODEX_HOME as the config directory itself", () => { + process.env.CLAUDE_CONFIG_DIR = "/somewhere/claude-cfg"; + process.env.CODEX_HOME = "/somewhere/codex-cfg"; + expect(claudeConfigHome()).toBe("/somewhere/claude-cfg"); + expect(codexConfigHome()).toBe("/somewhere/codex-cfg"); + }); + + it("treats FACTORY_HOME_OVERRIDE as a HOME with .factory appended", () => { + // This is the asymmetry the module exists for. Droid resolves + // `join($R(), ".factory")` where $R() is FACTORY_HOME_OVERRIDE || homedir(), + // so the var names the parent, not the config directory — the opposite of + // the other two. Getting it wrong makes ADE read a directory the spawned + // droid process never touches. + process.env.FACTORY_HOME_OVERRIDE = "/somewhere/fake-home"; + expect(factoryConfigHome()).toBe(path.join("/somewhere/fake-home", ".factory")); + }); + + it("prefers an explicit homeDir over homedir(), and the env var over both", () => { + expect(factoryConfigHome({ homeDir: "/explicit" })).toBe(path.join("/explicit", ".factory")); + process.env.FACTORY_HOME_OVERRIDE = "/env-wins"; + expect(factoryConfigHome({ homeDir: "/explicit" })).toBe(path.join("/env-wins", ".factory")); + }); + + it("ignores blank and whitespace-only overrides", () => { + process.env.CODEX_HOME = " "; + process.env.FACTORY_HOME_OVERRIDE = ""; + expect(codexConfigHome()).toBe(path.join(HOME, ".codex")); + expect(factoryConfigHome()).toBe(path.join(HOME, ".factory")); + }); + + it("reads the env object it is given rather than the ambient process env", () => { + process.env.CODEX_HOME = "/ambient"; + expect(codexConfigHome({ env: { CODEX_HOME: "/injected" } })).toBe("/injected"); + expect(codexConfigHome({ env: {} })).toBe(path.join(HOME, ".codex")); + }); +}); diff --git a/apps/desktop/src/main/services/shared/providerConfigHomes.ts b/apps/desktop/src/main/services/shared/providerConfigHomes.ts index ec9d66db3..c21f7845d 100644 --- a/apps/desktop/src/main/services/shared/providerConfigHomes.ts +++ b/apps/desktop/src/main/services/shared/providerConfigHomes.ts @@ -2,7 +2,29 @@ import { homedir } from "node:os"; import path from "node:path"; /** - * Where each provider CLI keeps its user-level config. + * Provider config: where it lives, and who owns each key. + * + * THE RULE. ADE hands its settings to every provider SDK at the highest + * precedence tier available — above the user's own config files, and in some + * cases above their per-project config too. So ADE must name a key only when it + * genuinely owns it: there is ADE UI for it and ADE's value is the truth. + * Otherwise the key stays absent and the provider's own precedence resolves it. + * + * Absence is the only way to say nothing. A substituted default is a real value + * that wins, which is how ADE spent five providers silently overriding + * configuration the user had set. Verified per provider by live probe: + * + * Claude omit -> the user's settings.json applies; "Default" is a real style + * Codex omit -> config.toml service_tier applies; null forces "default" + * Droid omit -> ~/.factory/settings.json applies, per key; null wedges the + * RPC for 30s, so omit, never null + * Cursor three states — absent lets ~/.cursor/sandbox.json decide, an + * explicit false skips the file entirely + * OpenCode OPENCODE_CONFIG_CONTENT deep-merges last, so any key ADE names wins + * + * Each adapter states only its own non-derivable fact and points here. + * + * Where each provider CLI keeps its user-level config: * * Every one of these has an env override that the provider's own binary honours, * and the overrides do NOT share a shape — `CODEX_HOME` and `CLAUDE_CONFIG_DIR` From edf9875b4320270e85576e3e778573c9560a817b Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:33:36 -0400 Subject: [PATCH 09/18] =?UTF-8?q?quality:=20re-review=20pass=20=E2=80=94?= =?UTF-8?q?=20Windows=20path=20comparison=20and=20structural=20cleanups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-review's one behavioral finding: claudeRootsByPrecedence compared paths with `===`. On Windows the same directory is reachable through more than one spelling, so a hand-typed CLAUDE_CONFIG_DIR differing only in drive-letter or user-name case would both fail to match the real home root and let one directory enter the precedence list twice, shadowing the tier below it. Now routed through pathsEqual/pathKey, which the repo already had for exactly this. Structural findings applied: - planRequested is now derived from resolveDroidSdkInteractionMode rather than restating its spec rule. The `stated?.interactionMode === "spec"` gate was only correct because the two happened to agree; editing the resolver would have silently stopped emitting spec-mode config with no type error and no failing test. - The cursor log line called buildCursorSdkLocalRunOptions instead of reconstructing the directive from the options it had just built. - `stated` is explicitly typed, which drops an `as const`, and spreads directly. - Restored one trimmed sentence that was carrying a probed fact: when ADE does ask Cursor for a sandbox, a user policy still wins — the SDK only falls back to its own default when the user wrote none. That is stated nowhere else. - Adapter comments now name services/shared/providerConfigHomes.ts, so the rule they follow is reachable by grep from the files that follow it. - Dropped imports orphaned by the earlier fixes. Verified, not changed: the reasoning fields are assigned as possibly-undefined while autonomy/interaction are conditionally spread. These are identical on the wire — settings cross a process boundary as JSON and JSON.stringify drops undefined keys, which the live Droid probe confirmed (updateSettings with an undefined key is a no-op, while null wedges the RPC for 30s). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../main/services/chat/agentChatService.ts | 22 +++++++++---------- .../main/services/chat/claudeOutputStyles.ts | 19 +++++++++++----- .../src/main/services/chat/cursorSdkPolicy.ts | 6 +++++ .../src/main/services/chat/cursorSdkWorker.ts | 6 +---- .../main/services/chat/droidSdkProtocol.ts | 1 + .../src/main/services/chat/droidSdkWorker.ts | 2 ++ .../main/services/opencode/openCodeRuntime.ts | 1 + 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 971a48a7a..cfb9d36ff 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -35103,21 +35103,21 @@ export function createAgentChatService(args: { // generic interaction-mode enum — resolve it from droidPermissionMode and // let it win over the plan→spec mapping. const chosenMode = resolveSessionDroidPermissionModeOrNull(managed.session); - const planRequested = managed.session.interactionMode === "plan" - || managed.session.permissionMode === "plan"; + const planRequested = resolveDroidSdkInteractionMode(managed.session) === "spec"; // Mirrors the terminal path: droidSettingsJson omits sessionDefaultSettings // when permissionMode is null, letting the user's settings.json decide. - const stated = chosenMode !== null || planRequested || isOrchestrationLeadSession(managed.session) - ? { - autonomyLevel: resolveDroidSdkAutonomyLevel(chosenMode ?? "auto-low"), - interactionMode: chosenMode === "agi" - ? "agi" as const - : resolveDroidSdkInteractionMode(managed.session), - } - : null; + const stated: Pick | null = + chosenMode !== null || planRequested || isOrchestrationLeadSession(managed.session) + ? { + autonomyLevel: resolveDroidSdkAutonomyLevel(chosenMode ?? "auto-low"), + interactionMode: chosenMode === "agi" + ? "agi" + : resolveDroidSdkInteractionMode(managed.session), + } + : null; return { modelId, - ...(stated ?? {}), + ...stated, // Droid's own editor/terminal tools live outside ADE's toolset, so a lead // has to have them withheld natively as well. ...(isOrchestrationLeadSession(managed.session) diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts index 175d46063..bb263a473 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { parse as parseYaml } from "yaml"; import type { AgentChatClaudeOutputStyle, AgentChatClaudePlugin } from "../../../shared/types/chat"; +import { pathKey, pathsEqual } from "../shared/pathCompare"; import { claudeConfigHome } from "../shared/providerConfigHomes"; import { writeTextAtomic } from "../shared/utils"; @@ -214,21 +215,27 @@ function claudeRootsByPrecedence(cwd: string): string[] { const roots: string[] = []; const seen = new Set(); const addRoot = (root: string): void => { - if (seen.has(root)) return; - seen.add(root); + // Keyed, not raw: Windows reaches the same directory through more than one + // spelling, and a duplicate root would shadow the tier below it. + const key = pathKey(root); + if (seen.has(key)) return; + seen.add(key); roots.push(root); }; - const userRoot = claudeConfigHome({ homeDir: os.homedir() }); - const realHomeRoot = path.join(path.resolve(os.homedir()), ".claude"); + // Passed explicitly so that os.homedir() spies reach the shared helper, which + // imports `homedir` by name. + const homeDir = os.homedir(); + const userRoot = claudeConfigHome({ homeDir }); + const realHomeRoot = path.join(path.resolve(homeDir), ".claude"); // A lane normally sits under $HOME, so the ancestor walk reaches ~/.claude and // would rank it as a project tier ABOVE the user tier. That is wrong whenever // CLAUDE_CONFIG_DIR moved the user tier elsewhere: the stale real ~/.claude // would outrank the directory the CLI actually reads. - const skipRealHomeRoot = userRoot !== realHomeRoot; + const skipRealHomeRoot = !pathsEqual(userRoot, realHomeRoot); for (const root of ancestorClaudeRoots(cwd)) { - if (skipRealHomeRoot && root === realHomeRoot) continue; + if (skipRealHomeRoot && pathsEqual(root, realHomeRoot)) continue; addRoot(root); } addRoot(userRoot); diff --git a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts index 61581e949..8d374b637 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkPolicy.ts @@ -67,8 +67,14 @@ export type CursorSdkReadonlyTool = (typeof CURSOR_SDK_READONLY_TOOLS)[number]; * returns `insecure_none` without ever reading the user's ~/.cursor/sandbox.json, * while absent lets that file decide. So ADE needs three states, not two. * + * When ADE does ask for a sandbox ("enable"), a user policy still wins over + * ADE's own — the SDK only falls back to its workspace_readwrite default when + * the user has written no policy at all. + * * "disable" also covers the retry after a ConfigurationError, where the * environment cannot sandbox at all and the alternative is a hard failure. + * + * See services/shared/providerConfigHomes.ts for the rule this follows. */ export type CursorSdkSandboxDirective = "enable" | "disable" | "inherit"; diff --git a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts index 8233c1992..1f97a7419 100644 --- a/apps/desktop/src/main/services/chat/cursorSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/cursorSdkWorker.ts @@ -512,11 +512,7 @@ async function initWorker(init: CursorSdkWorkerInit): Promise<{ agentId: string; useHttp1ForAgent, mode: agentOptions.mode ?? null, autoReview: agentOptions.local?.autoReview === true, - // Absent is a third state, not a falsy "off" — logging a boolean here - // collapsed "disable" and "inherit" into the same line. - sandboxDirective: agentOptions.local?.sandboxOptions === undefined - ? "inherit" - : agentOptions.local.sandboxOptions.enabled ? "enable" : "disable", + sandboxDirective: buildCursorSdkLocalRunOptions(init.policy, { sandboxSupported }).sandboxDirective, tools: agentOptions.tools ?? null, disallowedTools: agentOptions.disallowedTools ?? null, }, diff --git a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts index e5f0b49ce..b74ba4944 100644 --- a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts +++ b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts @@ -26,6 +26,7 @@ export type DroidSdkSessionSettings = { * the SDK, and omission resolves per key — a live probe confirmed an omitted * key falls through to the user's file while any stated value outranks it. * Never send null: an explicit null wedges the Droid RPC for 30 seconds. + * See services/shared/providerConfigHomes.ts for the rule this follows. */ autonomyLevel?: DroidSdkAutonomyLevel; interactionMode?: DroidSdkInteractionMode; diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index aec43cafa..6a4aa05e6 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -49,6 +49,8 @@ async function getSdk(): Promise { return sdkModule; } +// Still accepts null: settings cross a process boundary as JSON, so the +// protocol type is a contract with the sender rather than a runtime guarantee. function coerceReasoning(value: DroidSdkReasoningEffort | null | undefined): DroidSdkTypes.ReasoningEffort | undefined { return value?.trim() ? value as DroidSdkTypes.ReasoningEffort : undefined; } diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts index eed026cee..8142a26dc 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts @@ -515,6 +515,7 @@ export function buildOpenCodeConfig(args: BuildOpenCodeConfigArgs): OpenCodeConf // documented default is true. `autoupdate` moved to OPENCODE_DISABLE_AUTOUPDATE // in the server env — ADE does pin the binary, but that does not need the // highest-precedence config slot. + // See services/shared/providerConfigHomes.ts for the rule this follows. return { ...(provider ? { provider } : {}), ...(args.mcp ? { mcp: args.mcp } : {}), From 0521fa5bb73e83394b751968df8b4e6c8ea8aa84 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:34:12 -0400 Subject: [PATCH 10/18] opencode: pin the isolated-lead autoupdate suppression with a test The fix had no coverage, and the failure mode is silent: buildIsolatedOpenCodeEnv rebuilds its env from scratch and drops every OPENCODE_* var, so anything set on the user path never reaches a lead. Without the flag the lead's server updates the binary ADE resolves and pins. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../src/main/services/opencode/openCodeServerManager.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/desktop/src/main/services/opencode/openCodeServerManager.test.ts b/apps/desktop/src/main/services/opencode/openCodeServerManager.test.ts index d5be6d196..934dee4cd 100644 --- a/apps/desktop/src/main/services/opencode/openCodeServerManager.test.ts +++ b/apps/desktop/src/main/services/opencode/openCodeServerManager.test.ts @@ -580,6 +580,10 @@ describe("openCodeServerManager", () => { expect(spec.env.XDG_RUNTIME_DIR).toBe("/tmp/ade-opencode-test-home/xdg-v1/runtime"); expect(spec.env.OPENCODE_CONFIG_DIR).toBe("/tmp/ade-opencode-test-home/xdg-v1/config/opencode"); expect(spec.env.OPENCODE_DISABLE_PROJECT_CONFIG).toBe("1"); + // This builder rebuilds the env from scratch and drops every OPENCODE_* var, + // so the suppression set on the user path does not reach an isolated lead — + // without it the lead's server self-updates the binary ADE pins. + expect(spec.env.OPENCODE_DISABLE_AUTOUPDATE).toBe("1"); expect(spec.env.OPENCODE_CONFIG_CONTENT).toBe(JSON.stringify(config)); expect(spec.env.ADE_OPENCODE_MANAGED).toBe("1"); expect(spec.env.ADE_OPENCODE_OWNER_PID).toBe(String(process.pid)); From e053c6b25a5647df6fb4b132714b58887e902ecd Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:48:53 -0400 Subject: [PATCH 11/18] droid: restore the only exit from Spec mode, and key one more path comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blast radius of the omission fix, caught on re-review. The Droid SDK has no exitSpecMode — `enterSpecMode` is one-way, and the unconditional `DroidInteractionMode.Auto` that the omission fix removed was ADE's only way back out. A session ADE had put into Spec, whose plan mode was later turned off while no permission mode was chosen, states nothing on the next update and stays read-only for the rest of its life with no UI indication. applySettings now tracks whether ADE itself entered Spec, and states Auto once on the way out before returning to saying nothing. That re-adds exactly one statement, in one bounded case, rather than reinstating the blanket override. The flag resets on init and teardown so a recycled worker cannot carry it. Also keys the output-style source labels through pathsEqual: the precedence walk now folds case, so a case-variant CLAUDE_CONFIG_DIR could put the canonical home spelling in the root list while the raw comparison still expected the variant, labelling user styles "project". Metadata only — nothing branches on it — but the two comparisons should not disagree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../src/main/services/chat/claudeOutputStyles.ts | 6 +++++- .../src/main/services/chat/droidSdkWorker.ts | 14 +++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts index bb263a473..981db6518 100644 --- a/apps/desktop/src/main/services/chat/claudeOutputStyles.ts +++ b/apps/desktop/src/main/services/chat/claudeOutputStyles.ts @@ -366,7 +366,11 @@ export function discoverClaudeOutputStyles(cwd: string): AgentChatClaudeOutputSt const cwdClaudeRoot = path.resolve(cwd, ".claude"); for (const root of roots) { const resolvedRoot = path.resolve(root); - const source = resolvedRoot === cwdClaudeRoot ? "project" : resolvedRoot === homeClaudeRoot ? "user" : "project"; + // Keyed, like the precedence walk: raw comparison mislabels a case-variant + // root as "project" on the platforms where the two are the same directory. + const source = pathsEqual(resolvedRoot, cwdClaudeRoot) + ? "project" + : pathsEqual(resolvedRoot, homeClaudeRoot) ? "user" : "project"; for (const style of discoverOutputStyleFiles(path.join(root, "output-styles"), source)) add(style); } diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index 6a4aa05e6..4d11f5ceb 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -24,6 +24,8 @@ type DroidSession = Awaited>; let sdkModule: DroidSdkModule | null = null; let initState: DroidSdkWorkerInit | null = null; let session: DroidSession | null = null; +// Set when ADE itself put this session into Spec mode; see applySettings. +let enteredSpecMode = false; const activeAborts = new Set(); let waiterSeq = 0; const permissionWaiters = new Map void>(); @@ -341,10 +343,18 @@ async function applySettings(settings: DroidSdkSessionSettings): Promise { specModeModelId: settings.specModeModelId?.trim() || settings.modelId, specModeReasoningEffort: coerceReasoning(settings.specModeReasoningEffort ?? settings.reasoningEffort), }); + enteredSpecMode = true; if (disabledToolIds?.length) await session.updateSettings({ disabledToolIds }); return; } - const updateInteractionMode = toDroidInteractionMode(sdk, settings.interactionMode); + // Omitting the mode leaves Droid's own setting alone, which is the point — + // except when ADE is the one that put this session into Spec. The SDK has no + // exitSpecMode, so the only way back out is to state a mode, and a plan + // session that later turns plan off states nothing. Say it once, for a spec + // ADE itself entered, then go back to saying nothing. + const updateInteractionMode = toDroidInteractionMode(sdk, settings.interactionMode) + ?? (enteredSpecMode ? sdk.DroidInteractionMode.Auto : undefined); + if (updateInteractionMode) enteredSpecMode = false; await session.updateSettings({ modelId: settings.modelId, ...(settings.autonomyLevel ? { autonomyLevel: settings.autonomyLevel as DroidSdkTypes.AutonomyLevel } : {}), @@ -356,6 +366,7 @@ async function applySettings(settings: DroidSdkSessionSettings): Promise { async function initWorker(init: DroidSdkWorkerInit): Promise { initState = init; + enteredSpecMode = false; const sdk = await getSdk(); const resumeId = init.resumeSessionId?.trim(); if (resumeId) { @@ -469,6 +480,7 @@ async function dispose(): Promise { await cancelRun().catch(() => undefined); await session?.close().catch(() => undefined); session = null; + enteredSpecMode = false; initState = null; } From f0f8c4a8c5174810f4071e5f60bab99bb99d8272 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:56:16 -0400 Subject: [PATCH 12/18] test: pin the Droid settings contract, and align plan mode across both paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing the tests exposed that the Droid omission does not fire in practice. ADE always carries a generic permissionMode, and legacyPermissionModeToDroid PermissionMode maps it onto a Droid mode, so chosenMode is effectively never null: a bare Droid session resolves to autonomyLevel "low", not to an omitted key. That is the ADE-owned half of the rule working as intended — the composer chip is real UI, so ADE's value should win — but it means the omission path is reachable only from launches that carry no permission mode at all. The tests now pin what actually happens rather than a claim that cannot be reached. Also found by the same tests: ADE's two Droid paths disagreed about plan mode. droidSettingsJson sends {interactionMode: spec, autonomyLevel: off} on the terminal path, while the SDK path sent spec alongside whatever the permission chip mapped to — "low" for a default session. Spec collapses Droid's compound autonomyMode and reads back as level "off", so the extra claim was discarded and behavior was unaffected, but the two paths should not state different things. The SDK path now sends "off" with spec, and the spec-mode fields stay gated on the stated interaction mode. Three tests added to the existing agentChatService suite rather than a new file: autonomy derived from ADE's chip, an explicitly chosen mode, and plan mapping onto spec with autonomy off. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/agentChatService.test.ts | 70 +++++++++++++++++++ .../main/services/chat/agentChatService.ts | 21 ++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 746cffc84..a0573b059 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -5988,6 +5988,76 @@ describe("createAgentChatService", () => { })).rejects.toThrow("transcript history without fork mode"); }); + it("derives Droid autonomy from ADE's own permission chip", async () => { + // ADE always carries a generic permissionMode, and it maps onto a Droid + // mode — so ADE does state autonomy here, deliberately. This is the + // ADE-owned half of the rule: there IS a control for it, so ADE's value + // wins. The omission path exists for launches that carry no permission + // mode at all (programmatic/mobile), not for this one. + const { service } = createService(); + const session = await service.createSession({ + laneId: "lane-1", + provider: "droid", + model: "claude-opus-4-6", + modelId: "droid/claude-opus-4-6", + }); + await service.sendMessage({ sessionId: session.id, text: "hello" }); + + await vi.waitFor(() => { + expect(mockState.droidAcquireCalls.length).toBeGreaterThan(0); + }); + const settings = mockState.droidAcquireCalls[0]?.settings as Record; + expect(settings.autonomyLevel).toBe("low"); + expect(settings.interactionMode).toBe("auto"); + // Never null: an explicit null wedges the Droid RPC for 30 seconds. + expect(settings.autonomyLevel).not.toBeNull(); + expect(settings.interactionMode).not.toBeNull(); + }); + + it("states the Droid autonomy the user picked explicitly", async () => { + const { service } = createService(); + const session = await service.createSession({ + laneId: "lane-1", + provider: "droid", + model: "claude-opus-4-6", + modelId: "droid/claude-opus-4-6", + droidPermissionMode: "auto-high", + }); + await service.sendMessage({ sessionId: session.id, text: "hello" }); + + await vi.waitFor(() => { + expect(mockState.droidAcquireCalls.length).toBeGreaterThan(0); + }); + const settings = mockState.droidAcquireCalls[0]?.settings as Record; + expect(settings.autonomyLevel).toBe("high"); + expect(settings.interactionMode).toBe("auto"); + expect(settings).not.toHaveProperty("specModeModelId"); + }); + + it("maps a Droid plan session onto spec mode with autonomy off", async () => { + // Plan must stay read-only. Spec dominates Droid's compound autonomyMode, + // and the spec-mode model fields have to ride along with it — they are + // gated on the stated interaction mode, so they cannot be emitted for a + // session that stated none. + const { service } = createService(); + const session = await service.createSession({ + laneId: "lane-1", + provider: "droid", + model: "claude-opus-4-6", + modelId: "droid/claude-opus-4-6", + interactionMode: "plan", + }); + await service.sendMessage({ sessionId: session.id, text: "plan this" }); + + await vi.waitFor(() => { + expect(mockState.droidAcquireCalls.length).toBeGreaterThan(0); + }); + const settings = mockState.droidAcquireCalls[0]?.settings as Record; + expect(settings.interactionMode).toBe("spec"); + expect(settings.autonomyLevel).toBe("off"); + expect(settings.specModeModelId).toBeTruthy(); + }); + it("refuses a cross-machine Droid fork with the portability message", async () => { installCleanCrossMachineGitFixture(); const { service } = createService(); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index cfb9d36ff..5d583a465 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -35108,12 +35108,21 @@ export function createAgentChatService(args: { // when permissionMode is null, letting the user's settings.json decide. const stated: Pick | null = chosenMode !== null || planRequested || isOrchestrationLeadSession(managed.session) - ? { - autonomyLevel: resolveDroidSdkAutonomyLevel(chosenMode ?? "auto-low"), - interactionMode: chosenMode === "agi" - ? "agi" - : resolveDroidSdkInteractionMode(managed.session), - } + ? ((): Pick => { + const interactionMode = chosenMode === "agi" + ? "agi" as const + : resolveDroidSdkInteractionMode(managed.session); + return { + // Spec collapses Droid's compound autonomyMode to "spec" and reads + // back as level "off", so pairing it with anything else is a claim + // Droid discards. droidSettingsJson already sends "off" for plan on + // the terminal path; state the same thing here. + autonomyLevel: interactionMode === "spec" + ? "off" + : resolveDroidSdkAutonomyLevel(chosenMode ?? "auto-low"), + interactionMode, + }; + })() : null; return { modelId, From 4d095c699e173800ffa222b0a498b082f124e131 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 06:00:04 -0400 Subject: [PATCH 13/18] test: make the Droid interaction-mode mapping testable and pin it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit droidSdkWorker.ts has zero exports and attaches a process.on("message") handler at import — it is a fork() entrypoint, so nothing in it can be unit tested and the re-materialisation bug it carried was unpinnable where it lived. Moved the pure mapping to droidSdkProtocol.ts, which is importable and already has a suite, and gave it the enum table rather than the SDK module so it stays a pure function. Two tests now pin the contract that undefined maps to undefined — the exact behavior whose absence let the worker restate a mode the service had deliberately omitted — and that every stated mode still maps to its enum value. No new test file: the assertions extend the existing droidSdkProtocol suite. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/droidSdkProtocol.test.ts | 24 ++++++++++++++++- .../main/services/chat/droidSdkProtocol.ts | 23 ++++++++++++++++ .../src/main/services/chat/droidSdkWorker.ts | 27 +++---------------- 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/main/services/chat/droidSdkProtocol.test.ts b/apps/desktop/src/main/services/chat/droidSdkProtocol.test.ts index 0020179a5..7336a01f6 100644 --- a/apps/desktop/src/main/services/chat/droidSdkProtocol.test.ts +++ b/apps/desktop/src/main/services/chat/droidSdkProtocol.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { droidDisabledToolIdsForCategories, droidMcpToolsToDisable } from "./droidSdkProtocol"; +import { + droidDisabledToolIdsForCategories, + droidInteractionModeValue, + droidMcpToolsToDisable, +} from "./droidSdkProtocol"; import { ORCHESTRATION_LEAD_DENIED_DROID_TOOL_CATEGORIES } from "../../../shared/orchestrationRuntimePolicy"; // Shape mirrors Droid's `session.listTools()` result. Ids are build-specific @@ -79,3 +83,21 @@ describe("droidMcpToolsToDisable", () => { ]); }); }); + +describe("droidInteractionModeValue", () => { + const table = { Auto: "AUTO", Spec: "SPEC", AGI: "AGI" } as const; + + it("returns undefined for an omitted mode so the user's settings decide", () => { + // The regression: the worker mapped undefined onto Auto, which restated the + // mode at the highest precedence and undid the omission the service had + // deliberately made. A live probe showed omission resolves each key from the + // user's own ~/.factory/settings.json. + expect(droidInteractionModeValue(table, undefined)).toBeUndefined(); + }); + + it("maps every stated mode onto its SDK enum value", () => { + expect(droidInteractionModeValue(table, "auto")).toBe("AUTO"); + expect(droidInteractionModeValue(table, "spec")).toBe("SPEC"); + expect(droidInteractionModeValue(table, "agi")).toBe("AGI"); + }); +}); diff --git a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts index b74ba4944..f4438ad7d 100644 --- a/apps/desktop/src/main/services/chat/droidSdkProtocol.ts +++ b/apps/desktop/src/main/services/chat/droidSdkProtocol.ts @@ -49,6 +49,29 @@ export type DroidSdkSessionSettings = { * `apply-patch-cli`, `create-cli`, …), so ADE selects by the category Droid * itself reports rather than pinning a brittle id list. */ +/** + * Undefined in, undefined out. An omitted interactionMode means "ADE has no + * opinion, let ~/.factory/settings.json decide" — materialising a default here + * would restate it at the highest precedence and undo the omission upstream. + * + * Takes the enum table rather than the SDK module so it stays a pure mapping. + */ +export function droidInteractionModeValue( + table: { Auto: T; Spec: T; AGI: T }, + mode: DroidSdkInteractionMode | undefined, +): T | undefined { + switch (mode) { + case "spec": + return table.Spec; + case "agi": + return table.AGI; + case "auto": + return table.Auto; + default: + return undefined; + } +} + export function droidDisabledToolIdsForCategories( tools: ReadonlyArray<{ id?: unknown; category?: unknown }>, categories: readonly DroidToolCategory[], diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index 4d11f5ceb..f513788a5 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -10,7 +10,7 @@ import type { DroidSdkWorkerRequest, DroidSdkWorkerResponse, } from "./droidSdkProtocol"; -import { droidDisabledToolIdsForCategories, droidMcpToolsToDisable } from "./droidSdkProtocol"; +import { droidDisabledToolIdsForCategories, droidInteractionModeValue, droidMcpToolsToDisable } from "./droidSdkProtocol"; import { loadDroidSdk } from "../ai/droidSdkLoader"; import { summarizeDroidAskUser } from "./droidSdkAskUser"; import { ensureDroidSpawnsAreWindowless } from "./droidSdkWindowsHide"; @@ -57,33 +57,12 @@ function coerceReasoning(value: DroidSdkReasoningEffort | null | undefined): Dro return value?.trim() ? value as DroidSdkTypes.ReasoningEffort : undefined; } -/** - * Undefined in, undefined out. An omitted interactionMode means "ADE has no - * opinion, let ~/.factory/settings.json decide" — materialising a default here - * would restate it at the highest precedence and undo the omission upstream. - */ -function toDroidInteractionMode( - sdk: DroidSdkModule, - mode: DroidSdkSessionSettings["interactionMode"], -): DroidSdkTypes.DroidInteractionMode | undefined { - switch (mode) { - case "spec": - return sdk.DroidInteractionMode.Spec; - case "agi": - return sdk.DroidInteractionMode.AGI; - case "auto": - return sdk.DroidInteractionMode.Auto; - default: - return undefined; - } -} - function sessionOptions( sdk: DroidSdkModule, init: DroidSdkWorkerInit, settings: DroidSdkSessionSettings, ): DroidSdkTypes.CreateSessionOptions { - const interactionMode = toDroidInteractionMode(sdk, settings.interactionMode); + const interactionMode = droidInteractionModeValue(sdk.DroidInteractionMode, settings.interactionMode); return { cwd: init.laneRoot, execPath: init.droidPath, @@ -352,7 +331,7 @@ async function applySettings(settings: DroidSdkSessionSettings): Promise { // exitSpecMode, so the only way back out is to state a mode, and a plan // session that later turns plan off states nothing. Say it once, for a spec // ADE itself entered, then go back to saying nothing. - const updateInteractionMode = toDroidInteractionMode(sdk, settings.interactionMode) + const updateInteractionMode = droidInteractionModeValue(sdk.DroidInteractionMode, settings.interactionMode) ?? (enteredSpecMode ? sdk.DroidInteractionMode.Auto : undefined); if (updateInteractionMode) enteredSpecMode = false; await session.updateSettings({ From dc04bb5ddfe4291262c480f6106e86a26dc5e015 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 06:01:30 -0400 Subject: [PATCH 14/18] docs+tui: record the provider config-ownership rule and honour CLAUDE_CONFIG_DIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docs now carry the rule itself rather than a list of what changed. agent-routing gains "Provider config ownership" — the rule, plus a per-provider table of what omitting a key does and what an explicit null/false does, each row verified against a live runtime rather than read off a schema — and "Provider config homes" for the three env overrides that do not share a shape. Source file maps and the affected feature READMEs point at it, so provider #6 starts there instead of reverse-engineering the rule from a Cursor comment. TUI parity, both the same bug class this branch is about: - claudeHomePath hardcoded ~/.claude, so the TUI read keybindings, statusLine, vim mode, and agents from a directory Claude Code is not using whenever CLAUDE_CONFIG_DIR moved it. Now goes through claudeConfigHome. - formatOutputStyles keyed the active row off a session value that is now null until a settings file names one, so the listing would have highlighted nothing. It falls back to "Default" for display only, matching what the desktop /output-style handler shows, and never writes it back. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../tuiClient/__tests__/keybindings.test.ts | 24 +++ apps/ade-cli/src/tuiClient/app.tsx | 6 +- .../src/tuiClient/keybindings/index.ts | 12 +- docs/ARCHITECTURE.md | 2 +- docs/features/agents/README.md | 10 + docs/features/chat/README.md | 18 +- docs/features/chat/agent-routing.md | 193 +++++++++++++++++- .../features/terminals-and-sessions/README.md | 10 +- .../external-session-import.md | 9 +- .../pty-and-sessions.md | 22 +- .../runtime-isolation.md | 11 +- 11 files changed, 287 insertions(+), 30 deletions(-) diff --git a/apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts b/apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts index 09b1da055..6ab0d32be 100644 --- a/apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts +++ b/apps/ade-cli/src/tuiClient/__tests__/keybindings.test.ts @@ -1,5 +1,9 @@ +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; import { + claudeHomePath, + defaultKeybindingsPath, dispatchKeybinding, keybindingsEditorCommand, keypressToChord, @@ -164,3 +168,23 @@ describe("keybindings", () => { }); }); }); + +describe("claudeHomePath", () => { + it("follows CLAUDE_CONFIG_DIR, which is the directory the claude binary reads", () => { + const previous = process.env.CLAUDE_CONFIG_DIR; + try { + delete process.env.CLAUDE_CONFIG_DIR; + expect(claudeHomePath("settings.json")).toBe(path.join(os.homedir(), ".claude", "settings.json")); + + process.env.CLAUDE_CONFIG_DIR = path.join(os.tmpdir(), "ade-claude-config-dir"); + // The env var names the config directory itself — no ".claude" is appended. + expect(defaultKeybindingsPath()).toBe( + path.join(os.tmpdir(), "ade-claude-config-dir", "keybindings.json"), + ); + expect(claudeHomePath("agents")).toBe(path.join(os.tmpdir(), "ade-claude-config-dir", "agents")); + } finally { + if (previous === undefined) delete process.env.CLAUDE_CONFIG_DIR; + else process.env.CLAUDE_CONFIG_DIR = previous; + } + }); +}); diff --git a/apps/ade-cli/src/tuiClient/app.tsx b/apps/ade-cli/src/tuiClient/app.tsx index 48e7558d6..d429cfefa 100644 --- a/apps/ade-cli/src/tuiClient/app.tsx +++ b/apps/ade-cli/src/tuiClient/app.tsx @@ -1719,7 +1719,11 @@ export function isNewChatSetupPane(pane: RightPaneContent): boolean { function formatOutputStyles(styles: Awaited>, activeStyle?: string | null): string { if (!styles.length) return "No Claude output styles were found."; - const activeKey = activeStyle?.trim().toLowerCase() ?? ""; + // A session carries no output style until a settings file names one — ADE no + // longer substitutes a value into the SDK options. For the *listing* only, an + // unset selection still reads as Claude's own "Default", which is what the + // desktop's `/output-style` handler shows. Display, never written back. + const activeKey = (activeStyle?.trim() || "Default").toLowerCase(); return [ "Claude output styles:", "", diff --git a/apps/ade-cli/src/tuiClient/keybindings/index.ts b/apps/ade-cli/src/tuiClient/keybindings/index.ts index 89839d9e8..4b8108c4d 100644 --- a/apps/ade-cli/src/tuiClient/keybindings/index.ts +++ b/apps/ade-cli/src/tuiClient/keybindings/index.ts @@ -2,6 +2,7 @@ import { spawn } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { claudeConfigHome } from "../../../../desktop/src/main/services/shared/providerConfigHomes"; export const CLAUDE_KEYBINDING_CONTEXTS = [ "Global", @@ -232,8 +233,17 @@ const DEFAULT_CONFIG = { bindings: [], }; +/** + * The user-level Claude config directory the `claude` binary itself reads. + * + * `CLAUDE_CONFIG_DIR` names that directory outright, so hardcoding `~/.claude` + * makes the TUI read keybindings, statusLine, vim mode, and agents from a + * directory Claude Code is not using. `os.homedir()` is passed explicitly + * because the shared helper imports `homedir` by name, which a `vi.spyOn(os, + * "homedir")` in the suites below would otherwise miss. + */ export function claudeHomePath(...segments: string[]): string { - return path.join(os.homedir(), ".claude", ...segments); + return path.join(claudeConfigHome({ homeDir: os.homedir() }), ...segments); } export function defaultKeybindingsPath(): string { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1fdaa8fef..17b56c741 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -991,7 +991,7 @@ Most services described here live under `apps/desktop/src/main/services/ | `runtime/` | `tempCleanupService.ts`, `processRegistryService.ts`, `machineStateMigration.ts`, `packagedNodePath.ts`, `lastFailureStore.ts`, `projectRecoveryService.ts` | Runtime temp cleanup. `processRegistryService` is the per-process heartbeat registrar against machine-local `runtime_processes` (see §3.4); reconcile/dispose paths in `sessionService` and `ptyService` consult live and known owner sets before sweeping `terminal_sessions` rows so sibling processes and synced remote-machine owners are preserved. `machineStateMigration` carries one-shot migrations of the per-machine state files under `~/.ade/`. `packagedNodePath.ts` centralizes the `Resources/app*.asar(.unpacked)/node_modules` search path used by packaged runtime children. `lastFailureStore` records bounded typed project/machine failure reports and crash-loop backoff; `projectRecoveryService` runs the brain-independent diagnose/repair sequence behind `ade.recovery.*` (see [features/storage-and-recovery/README.md](./features/storage-and-recovery/README.md)). | | `search/` | `searchService.ts`, `searchIndexDb.ts`, `searchQueryParser.ts`, `searchRanking.ts`, `terminalChunking.ts`, `searchServiceWiring.ts` | Universal search over chat/terminal/PR/commit/branch text via a disposable FTS5 index (`.ade/cache/search-index.db`, never inside `ade.db`, never synced), unioned at query time with delegated lanes/files/artifacts/Linear. Accepted chat messages own the searchable document while processed/unprocessed events remain lifecycle-only; an exact `session:` query reads live ownership state, overrides stale same-document FTS metadata, and deduplicates totals. Debounced off-hot-path ingestion with cursor-based incremental reads, deterministic ranking tiers, and the `search` ADE action domain (`query`/`indexStatus`/`rebuildIndex`). `searchServiceWiring.ts` is shared with the `ade` runtime so wiring can't drift. See [features/search/README.md](./features/search/README.md). | | `sessions/` | `sessionService.ts`, `sessionDeltaService.ts`, `chatSessionProjection.ts`, `settleTerminalSession.ts` | Terminal session CRUD, post-session delta computation, provider-chat runtime projection onto resumable terminal rows, and the atomic settle/dismiss-pending-input boundary shared by IPC and ADE actions. | -| `shared/` | `utils.ts`, `imageDimensions.ts`, `remoteTrackingBranch.ts`, `packLegacyUtils.ts`, `transcriptInsights.ts` | Cross-domain utilities, including shared record guards, remote tracking-branch refresh, and PNG/JPEG dimension parsing used by App Control and iOS Simulator capture paths. | +| `shared/` | `utils.ts`, `imageDimensions.ts`, `remoteTrackingBranch.ts`, `packLegacyUtils.ts`, `transcriptInsights.ts`, `pathCompare.ts`, `providerConfigHomes.ts` | Cross-domain utilities, including shared record guards, remote tracking-branch refresh, case/separator-correct path comparison, and PNG/JPEG dimension parsing used by App Control and iOS Simulator capture paths. `providerConfigHomes.ts` is the one place that knows where each provider CLI keeps its user-level config — `claudeConfigHome` (`CLAUDE_CONFIG_DIR`), `codexConfigHome` (`CODEX_HOME`), `factoryConfigHome` (`FACTORY_HOME_OVERRIDE`, which replaces HOME rather than naming the directory) — and it carries the config-ownership rule every provider adapter follows: name a key only when ADE owns it, because ADE's settings land above the user's own config files. Consumed by the chat adapters, `pty/ptyService.ts`, `externalSessions/`, and `chat/droidModelsDiscovery.ts`. See [Provider config ownership](./features/chat/agent-routing.md#provider-config-ownership). | | `state/` | `kvDb.ts`, `crsqliteExtension.ts`, `dbMaintenanceApi.ts`, `globalState.ts`, `projectState.ts`, `onConflictAudit.ts` | SQLite schema + open (WAL + `synchronous = NORMAL`), CRR extension loader, global state file, per-project state init. The desktop's Electron user-data `ade-state.json` holds machine-local shell state, including `AutoUpdatePreferences`; missing or malformed preference fields normalize to automatic installation off and idle-only safety on. `kvDb` also attaches the optional `maintenance` (`DbMaintenanceApi`) handle — retention prunes, zero-peers-only cr-sqlite compaction, and fragmentation-gated vacuum — whose interface and shared retention constants live in `dbMaintenanceApi.ts` and are invoked by the storage doctor. `globalState.upsertRecentProject` accepts `preserveRecentOrder` so reactivating an already-known project (by app focus, deep link, etc.) refreshes its `lastOpenedAt` in place instead of jumping it to the front of the recents list. Recent projects use stable keys: local rows are keyed by absolute root path, remote rows by `remote::`, so a remote path string never collides with a local project. Pinned rows are retained above normal recency ordering and survive beyond the cap. `model_picker_favorites` and `model_picker_recents` are per-project CRR tables shared by desktop, TUI, and iOS; they are primary-key-only so CRR can convert them, with the recents cap enforced in `modelPickerStore.ts`. `AdeDb.sync.discardUnpublishedChangesForTables(tableNames)` lets a service clear local CRR state for specific tables without leaking those clears to sync peers — it records the cleared tables and `through_db_version` in the local-only `local_crr_change_suppressions` table, and `exportChangesSince` filters local-site rows for those tables at or below that version on the way out. The local-only excluded set (still kept out of replication) includes that suppression table itself, the snapshot caches, `local_worktree_residual_cleanups`, `pr_auto_link_ignores`, `pull_request_ai_summaries`, and `runtime_processes`. `crsql_changes` DELETE statements run through a helper that swallows the read-only-table error the cr-sqlite extension raises when a CRR-managed table is wiped, with a `db.crr_changes_cleanup_skipped` warn log instead of failing the migration. | | `sync/` | `syncService.ts`, `syncHostService.ts`, `syncPeerService.ts`, `syncRemoteCommandService.ts`, `syncProtocol.ts`, `deviceRegistryService.ts`, `syncPairingStore.ts` | **Thin delegation to the ADE runtime's sync service.** The authoritative sync service now lives in `apps/ade-cli/src/services/sync/`; the desktop main-process instances default to a non-host viewer role for legacy state and tests. The old in-process host is disabled unless `ADE_ENABLE_DESKTOP_SYNC_HOST=1` (diagnostics only). Wire formats — WebSocket envelope, remote command routing, device registry, pairing secrets — are the same across both implementations. Viewer joins clear the local `devices` + `sync_cluster_state` rows and then call `db.sync.discardUnpublishedChangesForTables(["devices", "sync_cluster_state"])` so the resulting DELETE rows do not leak back to other peers; the peer client follows up with `syncPeerService.acknowledgeLocalDbVersion()` to advance the outbound cursor past the suppressed range. | | `tests/` | `testService.ts` | Test-suite execution + run history. | diff --git a/docs/features/agents/README.md b/docs/features/agents/README.md index 15924fe43..ddc047aa6 100644 --- a/docs/features/agents/README.md +++ b/docs/features/agents/README.md @@ -319,6 +319,16 @@ configuration, and MCP servers. Only an OpenCode orchestration lead receives ADE's isolated configuration and ADE-owned MCP lease. Other providers apply their equivalent lead gate without changing ordinary-chat configuration. +That is one instance of a rule every provider adapter follows: ADE's settings +land at the highest precedence tier each SDK offers, so ADE names a config key +only when it genuinely owns it and leaves everything else absent for the +provider's own precedence to resolve. The isolated orchestration-lead server is +where the rule needs care, because `buildIsolatedOpenCodeEnv` rebuilds the +environment from scratch and drops every inherited `OPENCODE_*` variable — the +lead's server therefore sets `OPENCODE_DISABLE_AUTOUPDATE=1` itself rather than +inheriting it, so it cannot self-update the binary ADE pinned. See +[Provider config ownership](../chat/agent-routing.md#provider-config-ownership). + ## Smart memory and reconstruction The CTO's durable memory lives in files under `.ade/cto/` (`MEMORY.md`, `thread-state.md`, `daily/.md`) owned by `ctoMemoryService`. A deterministic flush writes the rolling summary before compaction and before model switches; a best-effort LLM upgrade refines it. `refreshReconstructionContext()` re-injects identity plus memory after compaction and switches. Full details in [Identity and Personas](identity-and-personas.md#smart-memory-system). diff --git a/docs/features/chat/README.md b/docs/features/chat/README.md index efcf2ceaf..a951ed23b 100644 --- a/docs/features/chat/README.md +++ b/docs/features/chat/README.md @@ -51,8 +51,9 @@ for its separate RPC, sync, storage, and UI contracts. | `apps/desktop/src/main/services/chat/chatEnvelopeSpliceRepair.ts` | Resume-time repair for historical ADE envelope streams written before Claude text fragments used the stable SDK message id. It detects only runs of at least three consecutive text envelopes in one turn with distinct message ids, rebuilds from SDK message text when possible (otherwise locally merges), preserves every other JSONL line verbatim, skips files over 64 MB, and rewrites atomically with a one-time `.splice.bak`. | | `apps/desktop/src/shared/claudeSessionQuota.ts` | Classifies Claude hard session-quota rejects (`session limit` / non-`allowed` `rate_limit_event`), parses reset clocks, and builds the sticky `claude_session_quota` `ade_card`. Approaching (`allowed_warning`) stays a quiet notice. | | `apps/desktop/src/main/services/chat/claudeSubprocessReaper.ts` | Tracks Claude SDK subprocesses and tears them down on runtime shutdown, plus a tmpdir registry (`ade-claude-subprocesses.json`) so a crashed owner's children are reaped at the next startup. **Both platforms kill the tree, not the leaf — and note the asymmetry, because it inverts the usual assumption: Windows was the platform that had most of this right and POSIX was the weak one.** Windows already routed its SIGTERM and its stale-registry reap through `taskkill /T /F` (`terminateProcessTree` / `killWindowsProcessTree`); macOS and Linux sent a single-pid signal on *every* path and orphaned everything below it. Do not "fix" this back toward the single-pid form on the assumption that the Unix path is the mature one. An SDK process owns 2-4 MCP servers with children of their own, so signalling one pid leaves the fan-out running and a later SIGKILL orphans it permanently: Windows now uses `taskkill /T /F` for the SIGKILL escalation too — that one branch was still a leaf `child.kill("SIGKILL")`, which is exactly what turns a surviving tree into a permanently orphaned one — and POSIX spawns the child `detached` so it leads its own process group and signals `-pid`, falling back to the child handle and then the bare pid. For a stale registry record — no live handle — one `ps -o pgid=,etime=,command=` read answers the identity question Windows puts to `tasklist` (does the live command line still look like the recorded process), plus two Windows has no equivalent for: whether the pid has been alive longer than the record that describes it (a pid younger than its own record was recycled; a minute of clock slack before that verdict), and whether it leads its own group, so the tree can be taken in one signal. Interpreter names (`node`, `electron`, `ade`) are not identity evidence, and the reaper never signals its own pid — both guards exist because a bad accept now force-kills a whole group rather than one process. An unreadable answer still gets reaped as a single pid, because leaking a Claude subprocess is the worse failure. `recordsForSession` backs the live-process line in `ade session show`. | -| `apps/desktop/src/main/services/chat/claudeOutputStyles.ts` | Discovers Claude output styles and plugins from project/user roots. Project roots are walked directly, while user-installed marketplace plugins are loaded only from Claude's installed-plugin registry when enabled in settings, so cache/source copies do not leak into ADE sessions. | +| `apps/desktop/src/main/services/chat/claudeOutputStyles.ts` | Discovers Claude output styles and plugins from project/user roots, and reads settings values (`readClaudeOutputStyleSelection`, `readClaudeWorkflowSizeGuideline`) across the same file chain the Agent SDK itself resolves, returning `null` when no file declares the key. Project roots are walked directly, while user-installed marketplace plugins are loaded only from Claude's installed-plugin registry when enabled in settings, so cache/source copies do not leak into ADE sessions. The user root and the plugin registry both come from `claudeConfigHome()`, so `CLAUDE_CONFIG_DIR` is honoured; the real `~/.claude` is dropped from the ancestor walk when that variable moved the user tier elsewhere, and roots are de-duplicated through `pathKey`/`pathsEqual` for Windows. | | `apps/desktop/src/main/services/chat/markdownSlashCommandDiscovery.ts` | Shared markdown-based slash command discovery engine. Provides frontmatter parsing, filesystem walking, command/agent/skill file discovery, command resolution, prompt expansion (`$ARGUMENTS` substitution), ancestor config root traversal, and deduplication helpers. Consumed by the provider-specific discovery modules (`claudeSlashCommandDiscovery`, `codexSlashCommandDiscovery`, `cursorSlashCommandDiscovery`). | +| `apps/desktop/src/main/services/shared/providerConfigHomes.ts` | Resolves each provider CLI's user-level config home (`claudeConfigHome`, `codexConfigHome`, `factoryConfigHome`) and carries the config-ownership rule every provider adapter follows. The three env overrides have different shapes: `CLAUDE_CONFIG_DIR` and `CODEX_HOME` name the config directory, `FACTORY_HOME_OVERRIDE` replaces the HOME that `.factory` is appended to. See [Provider config ownership](agent-routing.md#provider-config-ownership). | | `apps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.ts` | Discovers Claude-compatible command files plus Agent Skill entries. Delegates to `markdownSlashCommandDiscovery` for filesystem walking and markdown parsing. Command discovery walks ancestor and home `.claude/commands/**/*.md`; skill discovery uses `getAgentSkillRootCandidates()` so `.claude/skills`, `.agents/skills`, `.ade/skills`, `.cursor/skills`, `.codex/skills`, inherited env roots, and bundled ADE resources can surface `*/SKILL.md` command metadata. Consumed by `agentChatService` to enrich `chat.slashCommands` and provider prompt context with local command/skill metadata. | | `apps/desktop/src/main/services/chat/cursorSlashCommandDiscovery.ts` | Discovers Cursor-compatible slash commands from `.cursor/commands/**/*.md`, `.cursor/agents/**/*.md`, built-in Cursor subagents (`/explore`, `/bash`, `/browser`), and Agent Skill roots. Delegates to `markdownSlashCommandDiscovery` for filesystem walking. Consumed by `agentChatService` for the Cursor provider's `chat.slashCommands` list and by `slashCommandPromptExpansion` for Cursor prompt expansion. | | `apps/desktop/src/main/services/chat/projectSlashCommandDiscovery.ts` | Unified project-wide slash command discovery. Merges commands from Claude, Codex, and Cursor discovery modules into a single deduplicated list, filtering `/login`. Used by the ADE Code TUI's `discoverProjectSlashCommands` so all providers see the same cross-provider command catalog. | @@ -78,7 +79,7 @@ for its separate RPC, sync, storage, and UI contracts. | `apps/desktop/src/main/services/chat/droidSdkWorker.ts` | Node worker that hosts `@factory/droid-sdk`. Streams SDK events back to the main process and forwards permission / ask-user prompts back through the JSON-line protocol. | | `apps/desktop/src/main/services/chat/droidSdkProtocol.ts` | Worker IPC types: `DroidSdkSessionSettings` (autonomy level, interaction mode, reasoning effort), `DroidSdkReasoningEffort`, `DroidSdkPermissionRequest`/`Decision`, `DroidSdkAskUserRequest`/`Response`, `DroidSdkReady` (handshake with `availableModels`), and `DroidSdkSendPrompt`. | | `apps/desktop/src/main/services/chat/droidSdkEventMapper.ts` | Per-session `DroidSdkEventMapperState` + `mapDroidSdkMessageToChatEvents` / `mapDroidSdkRunResultToDoneEvent`. Tracks streaming text/thinking/image item ids, maps tool calls and results, maps `mission_worker_started` / `mission_worker_completed` notifications to provider-neutral subagent lifecycle events keyed by worker session id, surfaces image content as compact generation rows, and reports token usage. Replaces the deleted `droidAcpPool.ts` + `droidAcpEventMapper` path. | -| `apps/desktop/src/main/services/chat/droidModelsDiscovery.ts` | SDK-driven model probe (`listDroidModelsFromSdk`) plus the `~/.factory/config.json` custom-proxy merge. Normalizes the generic `opus` row to Opus 5 with its `high` default reasoning effort and Fast capability, while retired factory Claude ids still resolve forward (Sonnet 4.6 -> Sonnet 5, basic Opus 4.7 -> Opus 4.8) before descriptors reach desktop, mobile, or TUI model pickers. Exposes `discoverDroidSdkModelDescriptors` (alias for the legacy `discoverDroidCliModelDescriptors` while callers migrate). | +| `apps/desktop/src/main/services/chat/droidModelsDiscovery.ts` | SDK-driven model probe (`listDroidModelsFromSdk`) plus the `/config.json` custom-proxy merge (`~/.factory` unless `FACTORY_HOME_OVERRIDE` is set — see [Provider config homes](agent-routing.md#provider-config-homes)). Normalizes the generic `opus` row to Opus 5 with its `high` default reasoning effort and Fast capability, while retired factory Claude ids still resolve forward (Sonnet 4.6 -> Sonnet 5, basic Opus 4.7 -> Opus 4.8) before descriptors reach desktop, mobile, or TUI model pickers. Exposes `discoverDroidSdkModelDescriptors` (alias for the legacy `discoverDroidCliModelDescriptors` while callers migrate). | | `apps/desktop/src/main/services/chat/piSdkPool.ts` | Pi adapter. Forks `piSdkWorker` per session key, exposes `acquirePiSdkConnection` / `releasePiSdkConnection`, and proxies prompts, model/thinking changes, compaction, inventory reads, `login` / `cancelLogin`, and `respondToUi`. Also routes the reverse-RPC UI channel onto `bridge.onUiRequest` / `onUiNotice` / `onUiCancel`; when no `onUiRequest` handler is installed the pool answers `{ ok: false }` immediately, so an unattended worker fails closed instead of hanging a turn. | | `apps/desktop/src/main/services/chat/piSdkWorker.ts` | Node worker that hosts the user's own Pi installation (resolved at runtime, never statically imported). Owns the Pi agent session, the model runtime, `ModelRuntime.login`, tool assembly (`ask_user` plus approval-gated rebuilds of Pi's built-ins), extension binding, and the settings manager that pins `projectTrusted: false`. | | `apps/desktop/src/main/services/chat/piSdkProtocol.ts` | Worker IPC types and validators, at protocol version 2. Adds the `ui_request` / `ui_notice` / `ui_cancel` / `ui_response` frames and `login` / `login_cancel` on top of version 1, plus the `extensions` / `askUserTool` / `approvalTools` init flags and the `extensions` / `extensionsError` / `ungateableTools` fields on `PiSdkReady`. Every frame is validated in both directions. | @@ -475,11 +476,16 @@ Controls and summaries project this runtime state rather than owning it: envelopes remain the normal render backend. Deliberately not wired yet: SDK `SessionStore` as ADE's transcript backend and channels/external message origins. - Runtime launch fixes the Claude subagent/workflow policy explicitly + Runtime launch pins the Claude subagent policy explicitly (`CLAUDE_CODE_MAX_SUBAGENT_SPAWN_DEPTH=3`, - `CLAUDE_CODE_MAX_CONCURRENT_SUBAGENTS=20`, and - `workflowSizeGuideline: "medium"`) instead of inheriting defaults that can - change between bundled Claude Code releases. + `CLAUDE_CODE_MAX_CONCURRENT_SUBAGENTS=20`) rather than inheriting defaults that + can change between bundled Claude Code releases. `workflowSizeGuideline: + "medium"` is ADE's preferred default and is sent only when no settings file in + the resolved chain states one — ADE's `settings` object lands at flag tier, + above every `settings.json` the SDK reads, so a key ADE does not own has to + stay absent. The same rule governs `outputStyle`, which is omitted entirely + when no settings file names a style. See + [Provider config ownership](agent-routing.md#provider-config-ownership). - **Provider-agnostic sessions.** `AgentChatProvider` is one of `claude`, `codex`, `opencode`, `cursor`, `droid`, or a free-form string reserved for local providers. The service owns a pluggable adapter per provider (Claude diff --git a/docs/features/chat/agent-routing.md b/docs/features/chat/agent-routing.md index ef2862901..be94682ca 100644 --- a/docs/features/chat/agent-routing.md +++ b/docs/features/chat/agent-routing.md @@ -16,6 +16,7 @@ where the machinery lives. | `apps/desktop/src/shared/cliLaunch.ts` | Tracked provider CLI start/resume builders, including model/reasoning/permission flags and the canonical `computer_use` MCP overrides for Codex. | | `apps/desktop/src/main/services/ai/providerRuntimeHealth.ts` | Tracks provider readiness/auth/network failures so the UI can surface degraded states. | | `apps/desktop/src/main/services/ai/providerOptions.ts` | Normalises provider-native options (Claude permission mode, Codex approval + sandbox, OpenCode permission). | +| `apps/desktop/src/main/services/shared/providerConfigHomes.ts` | Where each provider CLI keeps its user-level config (`claudeConfigHome`, `codexConfigHome`, `factoryConfigHome`), and the canonical statement of the config-ownership rule below. Every adapter that reads or writes a provider config path goes through it. | | `apps/desktop/src/main/services/ai/authDetector.ts` | Discovers available credentials (CLI, API key, OAuth) and reports auth status. | | `apps/desktop/src/main/services/ai/codexExecutable.ts` / `droidExecutable.ts` | CLI resolution for runtimes that still need an external binary (looks on PATH, in the app bundle, then in configured install paths where supported). Claude uses the bundled Claude Agent SDK binary; Cursor and Droid run through embedded SDKs (`@cursor/sdk`, `@factory/droid-sdk`). | | `apps/desktop/src/main/services/ai/tools/systemPrompt.ts` | Adjusts the system prompt per mode (`chat`, `coding`, `planning`) and permission mode. | @@ -24,7 +25,7 @@ where the machinery lives. | `apps/desktop/src/main/services/chat/piSdkUiBridge.ts` | Worker-side half of the UI channel, deliberately free of Pi imports. Funnels Pi's three unrelated callback APIs — `AuthInteraction`, custom-tool `execute`, and an extension's `ExtensionUIContext` — into one never-rejecting `request()` that resolves to `null` when a card is dismissed, a turn aborts, or the worker is disposed. Also builds ADE's `ask_user` tool, the per-tool-call approval gate, and the extension UI context. | | `apps/desktop/src/main/services/ai/piInstallation.ts` | Resolves the user's Pi installation: CLI path, SDK package root/entry, agent dir, `auth.json` / models / settings paths, provider inventory, and a `blocker` string when the SDK cannot be used (missing package, or a Node older than `PI_SDK_MIN_NODE`). `sdkAvailable` and `cliAvailable` are independent — the CLI can be present while the SDK path is blocked. | | `apps/desktop/src/main/services/ai/piAuthService.ts` | In-app Pi sign-in. Enumerates the providers that can actually be signed into (`listPiLoginProviders`), runs one `startPiLogin` per provider on a dedicated inventory-only worker, relays Pi's prompts/notices through `addPiAuthStatusListener`, and answers them with `submitPiLoginPrompt`. Bounded at 10 minutes; `cancelPiLogin` stops a flow and releases its worker. Never reads, stores, or logs a credential. | -| `apps/desktop/src/main/services/chat/droidModelsDiscovery.ts` | Droid model discovery: probes the live SDK via `createSession({ execPath })` to read `initResult.availableModels`, normalizes `supportedReasoningEfforts` into `reasoningTiers`, and emits `droid/` descriptors via `createDynamicDroidCliModelDescriptor`. Droid fast choices are distinct model IDs, not ADE `serviceTiers`; custom (`~/.factory/config.json`) models are merged in. The legacy `DROID_DEFAULT_MODEL_IDS` constant has been removed — the SDK is the only source. Like Cursor, the cache is stale-while-revalidate: `markDroidModelCachesStale` ages it without dropping last-known-good rows, which are served past the 120s window (up to ~6h) while one background warm per freshness window refreshes them, so an unauthenticated/mid-reauth droid isn't handed a session per passive read. | +| `apps/desktop/src/main/services/chat/droidModelsDiscovery.ts` | Droid model discovery: probes the live SDK via `createSession({ execPath })` to read `initResult.availableModels`, normalizes `supportedReasoningEfforts` into `reasoningTiers`, and emits `droid/` descriptors via `createDynamicDroidCliModelDescriptor`. Droid fast choices are distinct model IDs, not ADE `serviceTiers`; custom models from `/config.json` (`~/.factory` unless `FACTORY_HOME_OVERRIDE` is set) are merged in. The legacy `DROID_DEFAULT_MODEL_IDS` constant has been removed — the SDK is the only source. Like Cursor, the cache is stale-while-revalidate: `markDroidModelCachesStale` ages it without dropping last-known-good rows, which are served past the 120s window (up to ~6h) while one background warm per freshness window refreshes them, so an unauthenticated/mid-reauth droid isn't handed a session per passive read. | ## Supported providers @@ -268,6 +269,159 @@ project on the machine and a root-wide token would make one lane's starting Pi chat block every other lane's. Pi ignores the file; all of its own scans filter on `.jsonl`. +## Provider config ownership + +ADE hands its settings to every provider SDK at the **highest precedence tier +that SDK offers** — above the user's own `settings.json` / `config.toml` / +`opencode.json`, and in some cases above their per-project config too. So the +rule for every adapter is: + +> Name a config key only when ADE genuinely owns it — there is ADE UI for it and +> ADE's value is the truth. Otherwise leave the key absent and let the +> provider's own precedence resolve it. + +Absence is the only way to say nothing. A substituted default is a real value +that wins, and a value ADE never surfaced is one the user cannot notice or undo. +The rule and the probe results behind it live in +`apps/desktop/src/main/services/shared/providerConfigHomes.ts`; each adapter +states only its own non-derivable fact and points there. **Provider #6 starts +here.** + +What "absent" means differs per provider, and each row was verified against a +live runtime rather than read off a schema: + +| Provider | Where ADE's settings land | Omitting a key | Explicit `null` / `false` | +|---|---|---|---| +| Claude | Agent SDK `settings` — flag tier, above every `settings.json` the SDK reads | the user's settings chain applies | `"Default"` is a real output style, not "no style" | +| Codex | `thread/start` + `turn/start` JSON-RPC args | `config.toml`'s `service_tier` applies | `null` reports `"default"` — a real downgrade | +| Droid | `createSession` / `updateSettings` SDK options | `~/.factory/settings.json` applies, resolved **per key** | `null` wedges the Droid RPC for 30 s — never send it | +| Cursor | `local.sandboxOptions` on the SDK agent options | `~/.cursor/sandbox.json` decides | `false` returns `insecure_none` without ever reading that file | +| OpenCode | `OPENCODE_CONFIG_CONTENT` | the user's `opencode.json` applies | n/a — this env var deep-merges **last**, so any key ADE names wins | + +### Provider config homes + +`providerConfigHomes.ts` also resolves where each CLI keeps its user-level +config, because every one has an env override the provider's own binary honours +and **the overrides do not share a shape**: + +| Helper | Env override | Shape | Default | +|---|---|---|---| +| `claudeConfigHome()` | `CLAUDE_CONFIG_DIR` | names the config **directory** | `~/.claude` | +| `codexConfigHome()` | `CODEX_HOME` | names the config **directory** | `~/.codex` | +| `factoryConfigHome()` | `FACTORY_HOME_OVERRIDE` | replaces the **HOME** that `.factory` is appended to | `~/.factory` | + +Hardcoding `~/.codex` or `~/.factory` makes ADE read a different directory than +the process it spawns, so ADE and the CLI disagree about the user's +configuration inside a single session. Every path into a provider config home +goes through these helpers: chat adapters, the Droid custom-model merge in +`droidModelsDiscovery.ts`, PTY session recovery in `ptyService.ts`, and +external-session discovery (`providerSessionHandles.ts`, `discoverDroid.ts`). +`homeDir` is passed explicitly by callers that already resolved a home of their +own; everything else resolves `homedir()` inside the helper. + +### What each adapter states + +**Claude.** ADE sends `enabledPlugins` (the CLI merges it per plugin key rather +than replacing the map) and `fastMode` (the composer's Fast chip owns it). +`outputStyle` is sent only when a settings file actually names one, and +`workflowSizeGuideline: "medium"` only when no settings file states one — ADE's +preferred default, supplied rather than imposed. +`readClaudeOutputStyleSelection` and `readClaudeWorkflowSizeGuideline` +(`claudeOutputStyles.ts`) resolve a key across the same files, in the same +order, that the SDK itself resolves with `settingSources: ["user", "project", +"local"]` — lane `settings.local.json`, lane `settings.json`, each ancestor +root, then the user root — and return `null` when no file declares it. +Two traps this closes: substituting `"Default"` suppresses a globally +configured style, and materialising a fallback onto the session record makes it +read back as a real choice on the next launch, pinning it forever. The session's +own cached value is therefore consulted *after* the settings files, not before. + +The precedence walk honours `CLAUDE_CONFIG_DIR` as well, and so does the plugin +registry (`/plugins/installed_plugins.json`). A lane normally +sits under `$HOME`, so the ancestor walk would reach the real `~/.claude` and +rank it as a *project* tier above the user tier; when `CLAUDE_CONFIG_DIR` has +moved the user tier elsewhere, that stale directory would outrank the one the +CLI actually reads, so the real `~/.claude` is skipped from the ancestor walk in +that case. Root de-duplication and the project/user source labelling both +compare through `pathKey` / `pathsEqual` rather than raw strings, because +Windows reaches the same directory through more than one spelling and a +duplicate root would shadow the tier below it. + +**Codex.** Service tier is stated only when the Fast chip is on +(`serviceTier: "fast"`); otherwise the key is omitted so `config.toml`'s +`service_tier` resolves. Fast-off cannot mean "force default" either: `fastMode` +is persisted only when true and rehydrated as `persisted?.fastMode === true`, so +`false` is indistinguishable from never-set. The app-server re-resolves per +request, so a turn sent after the toggle goes off inherits the config again. + +Reasoning effort travels **per thread** (`codexThreadConfigArgs`), never as a +`-c model_reasoning_effort=…` flag on the `codex app-server` process. That flag +outranks the user's `config.toml` and applies to every thread on that +app-server, not just the chat that set it. + +**Droid.** `resolveSessionDroidPermissionModeOrNull` returns the mode the user +actually chose, or `null`. Droid has no "use my config" mode — `cliLaunch` +rejects `config-toml` for it — so `null` is the only way ADE can say nothing, +and `autonomyLevel` / `interactionMode` are then both omitted from the SDK +options, each resolving independently from `~/.factory/settings.json`. This +matters because Droid's own documented default is `autonomyLevel: "off"` +(read-only): a substituted `auto-low` fallback would hand out write access the +CLI would not. `normalizeSessionNativePermissionControls` deletes the field +rather than materialising a fallback, for the same reason as Claude's output +style. + +When ADE does state a mode, Spec pairs with `autonomyLevel: "off"` — Droid +collapses its compound autonomy mode to `spec` and reads it back as level `off`, +so anything else is a claim Droid discards — which matches what +`droidSettingsJson` already sends on the terminal path. + +Spec is the one place ADE has to speak up to stay quiet. The SDK exposes no +`exitSpecMode`, so the only way out is to state a mode, and a plan session that +later turns plan off states nothing. The worker therefore tracks whether ADE +itself entered Spec (`enteredSpecMode` in `droidSdkWorker.ts`) and states `Auto` +exactly once to leave, then goes back to saying nothing. The flag is reset on +init and on dispose. + +`buildReady` reads the resolved model from `initResult.settings.modelId`. +`initResult.currentModelId` does not exist in `@factory/droid-sdk`; reading it +always yielded `null`. + +**Cursor.** The sandbox is a three-state directive, not a boolean: +`CursorSdkSandboxDirective = "enable" | "disable" | "inherit"` +(`cursorSdkPolicy.ts`). `inherit` omits `local.sandboxOptions` entirely so +`~/.cursor/sandbox.json` decides. `disable` sends `{ enabled: false }`, which +returns `insecure_none` without reading that file at all — which is exactly what +ADE's full-access mode means, and what the retry after a `ConfigurationError` +needs when the environment cannot sandbox and the alternative is a hard failure. +`enable` asks for a sandbox, and a user policy still wins over ADE's: the SDK +falls back to its own `workspace_readwrite` default only when the user has +written no policy at all. The directive, not a boolean, is what the local +permission fingerprint and the worker's ready payload carry, so a change between +the three states restarts the agent options. + +**OpenCode.** `OPENCODE_CONFIG_CONTENT` deep-merges last, so anything +`buildOpenCodeConfig` names outranks the user's `opencode.json` and only +managed/MDM config beats it. `share` and `snapshot` are therefore omitted — +neither has ADE UI, and forcing `snapshot: false` silently disabled OpenCode's +own `/undo` and `/revert`, whose documented default is `true`. `autoupdate` +moved out of config into `OPENCODE_DISABLE_AUTOUPDATE=1` on the server env: ADE +does pin the binary, but that does not need the highest-precedence config slot. +Both env builders set it — `buildIsolatedOpenCodeEnv` rebuilds the env from +scratch and drops every inherited `OPENCODE_*` var, so an orchestration lead's +isolated server would otherwise self-update the binary ADE pinned. + +Local provider blocks (`ollama`, `lmstudio`) are emitted only when the user +configured an endpoint or ADE discovered models for that family. An +ADE-invented `baseURL` merges over the endpoint in the user's own +`opencode.json`, repointing a configured remote host back at localhost. +`lmstudio` ships in OpenCode's provider catalog with its own npm package and +baseURL, so only `ollama` needs `npm` stated. + +ADE's four agent profiles are `hidden: true`. They are ADE's permission modes, +not agents the user should meet in Tab-cycle or `@`-autocomplete; without a +`mode` they default to `"all"` and show up in the picker. `ade-helper` uses +`steps: 1` (`maxSteps` is the deprecated spelling). + ## Permission modes Permission controls are provider-native. The session carries an abstract @@ -401,9 +555,11 @@ selected model, independent of provider. `AgentChatSession` carries `codexFastMode` is still accepted at boundaries for old rows and remote clients. -Codex forwards Fast as `serviceTier: "fast" | null` on every -`turn/start` and `thread/start` JSON-RPC call (an explicit `null` clears -any app-server default). Claude Fable and Opus descriptors advertise +Codex forwards Fast as `serviceTier: "fast"` on every `turn/start` and +`thread/start` JSON-RPC call, and **omits the key entirely** when Fast is off so +the user's `config.toml` `service_tier` resolves — an explicit `null` reports +`"default"`, which is a real downgrade ADE has no UI for. See +[Provider config ownership](#provider-config-ownership). Claude Fable and Opus descriptors advertise `serviceTiers: ["fast"]`; Claude chat sends the effective flag through the Agent SDK `settings.fastMode` layer, and Claude CLI launches/resumes pass `--settings '{"fastMode":true|false}'` so ADE can explicitly @@ -459,6 +615,28 @@ picker state matches the documented default; the explicit Codex | `edit` | Read/write allowed; bash gated. | | `full-auto` | Proceed without asking. | +Each mode is an `agent` entry in the config ADE ships through +`OPENCODE_CONFIG_CONTENT`, carrying an explicit `permission` block. +`OpenCodePermissionKey` in `openCodeRuntime.ts` names the keys ADE sets; the +OpenCode SDK's own type declares only five of them and absorbs the rest through +an index signature, so a misspelled key compiles and silently fails to apply +(`websearch` vs `web_search` is a pair this codebase has already been bitten by). + +- **`plan` denies `task`, not just `edit`.** A spawned subagent runs under its + own ruleset — OpenCode's `general` agent is `merge(base, todowrite: deny)`, + i.e. edit *allowed* — so leaving `task` open let a plan-mode session write + files through a child session. Plan has to mean plan. Plan also denies + `websearch` and `skill`; that is the supported spelling of what the deprecated + agent-level `tools` map used to express, since OpenCode desugars that map into + exactly these permission entries and an explicit `permission` block wins over + it. +- **`full-auto` states `read: "allow"` and `task: "allow"`.** Most ungated keys + resolve to `allow` from OpenCode's base `*` rule, but `read` does not: the base + ruleset asks before reading `*.env` / `*.env.*`, so full access still prompted. +- **`external_directory` stays `ask` even in `full-auto`.** That boundary is + ADE's lane worktree, not a permission tier the user picked — the same reason + the system prompt confines edits to the lane. + ### Pi Pi's built-in tool registry contains only `read`, `bash`, `edit`, and `write`, @@ -520,6 +698,11 @@ surfaces. `resolveCursorSdkPolicy` (`services/chat/cursorSdkPolicy.ts`) turns the ADE permission mode into a `CursorSdkPermissionPolicy`: chat mode, approval policy, sandbox mode, hard guards, orchestration-lead flag, and a `fullAuto` marker. +`buildCursorSdkLocalRunOptions` then reduces that policy to the SDK's local run +options, where the sandbox is a three-state `CursorSdkSandboxDirective` +(`enable` / `disable` / `inherit`) rather than a boolean — see +[Provider config ownership](#provider-config-ownership) for why absent and +`false` are not the same thing to `@cursor/sdk`. `fullAuto` is only the name of ADE's full-auto permission mode — it partitions the worker pool and labels logs. It is deliberately not wired to the Cursor SDK's `local.force` send option, which expires the currently active persisted @@ -560,7 +743,7 @@ translates the abstract value into the correct provider-native fields: - `claude`: `claudePermissionMode = "default" | "auto" | "plan" | "acceptEdits" | "bypassPermissions"`. The `auto` mode hands permission decisions to the SDK's automatic gate and surfaces in the desktop and `ade code` permission pickers alongside the existing modes. - `codex`: `codexApprovalPolicy` + `codexSandbox` pair. - `opencode`: `opencodePermissionMode = "plan" | "edit" | "full-auto"`. -- `droid`: `droidPermissionMode = "read-only" | "auto-low" | "auto-medium" | "auto-high"`. +- `droid`: `droidPermissionMode = "read-only" | "auto-low" | "auto-medium" | "auto-high" | "agi"`, or **absent** when the user has picked nothing. Absent is meaningful: it lets `~/.factory/settings.json` resolve autonomy, so nothing materialises a fallback onto the session. See [Provider config ownership](#provider-config-ownership). - `pi`: no provider-specific permission field — the abstract `permissionMode` *is* Pi's native field. It is read directly by `piSdkToolPolicyForPermissionMode` (chat) or `piToolsForPermissionMode` diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 4adf1f716..49f78f14c 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -280,11 +280,15 @@ and in tests. already-imported detection, active-session hints, CLI import into tracked PTYs, chat import delegation, cwd checks, and provider-specific resume/fork commands. The per-provider discovery modules scan Claude JSONL transcripts - under `~/.claude/projects`, Codex threads from the `~/.codex/state_5.sqlite` + under `/projects`, Codex threads from the `/state_5.sqlite` thread store (falling back to the `sessions/` rollout tree only when that database is unusable), Cursor artifacts under `~/.cursor/chats` and - `~/.cursor/projects`, Droid sessions under `~/.factory/sessions`, and - OpenCode through `opencode session list`. + `~/.cursor/projects`, Droid sessions under `/sessions`, and + OpenCode through `opencode session list`. The Claude/Codex/Droid roots come + from `services/shared/providerConfigHomes.ts` so `CLAUDE_CONFIG_DIR`, + `CODEX_HOME`, and `FACTORY_HOME_OVERRIDE` are honoured — and honoured with + their differing shapes; see + [Provider config homes](../chat/agent-routing.md#provider-config-homes). `claudeSessionTransplant.ts` performs the non-destructive Claude JSONL copy used when forking or importing a Claude session into a different lane cwd; `claudeLiveSessions.ts` reads Claude's own `sessions/.json` registry to diff --git a/docs/features/terminals-and-sessions/external-session-import.md b/docs/features/terminals-and-sessions/external-session-import.md index 4bdc25c96..d4066adeb 100644 --- a/docs/features/terminals-and-sessions/external-session-import.md +++ b/docs/features/terminals-and-sessions/external-session-import.md @@ -35,8 +35,9 @@ continuation metadata is recorded as soon as ADE knows the provider target. | `apps/desktop/src/main/services/externalSessions/discoverClaude.ts` | Discovers resumable Claude CLI JSONL transcripts under `CLAUDE_CONFIG_DIR` or `~/.claude/projects//.jsonl`; reads `ai-title`/custom titles, excludes SDK-origin transcripts, and collapses continuation chains to their leaf. | | `apps/desktop/src/main/services/externalSessions/discoverCodex.ts` | Discovers interactive Codex threads from `CODEX_HOME/state_5.sqlite` (default `~/.codex`): top-level threads only, fork continuations collapsed, enriched from the rollout JSONL under `sessions/YYYY/MM/DD/` and `session_index.jsonl`. Falls back to scanning rollout files when the thread store is unusable. | | `apps/desktop/src/main/services/externalSessions/discoverCursor.ts` | Groups every Cursor artifact — `~/.cursor/chats///store.db`, its `meta.json`, and `~/.cursor/projects//agent-transcripts/` (including `empty-window`) — by the bare conversation uuid, keeps the fullest copy of each, resolves cwd from `meta.json` before the md5/slug reverse-mappings, and excludes SDK `agent-` sessions. | -| `apps/desktop/src/main/services/externalSessions/discoverDroid.ts` | Discovers Factory Droid JSONL sessions under `~/.factory/sessions//`, one record per session id, using the `session_start` row for id/cwd/title. | +| `apps/desktop/src/main/services/externalSessions/discoverDroid.ts` | Discovers Factory Droid JSONL sessions under `/sessions//` (default `~/.factory`), one record per session id, using the `session_start` row for id/cwd/title. | | `apps/desktop/src/main/services/externalSessions/discoverOpenCode.ts` | Discovers OpenCode sessions by running `opencode session list --pure --format json --max-count ` in the requested/project cwd. | +| `apps/desktop/src/main/services/externalSessions/providerSessionHandles.ts` | Maps a provider session id back to the files that hold it. The Claude / Codex / Droid roots come from `shared/providerConfigHomes.ts` so `CLAUDE_CONFIG_DIR`, `CODEX_HOME`, and `FACTORY_HOME_OVERRIDE` are all honoured — and honoured with the right shape, since only the first two name a directory while `FACTORY_HOME_OVERRIDE` replaces the HOME that `.factory` is appended to. | | `apps/desktop/src/main/services/externalSessions/importedSessionStore.ts` | Machine-local durable log of every import (`/external-sessions/imported.json`), the only imported-marking source that survives deleting the ADE session and the only one that knows a fork's new provider id. | | `apps/desktop/src/main/services/externalSessions/claudeLiveSessions.ts` | Reads Claude's own live-session registry (`/sessions/.json`) and returns the session ids whose pid is still alive, for `possiblyActive`. | | `apps/desktop/src/main/services/externalSessions/claudeSessionTransplant.ts` | Non-destructive Claude transcript transplant. For forks it copies JSONL rows, rekeys `sessionId`, hard-links without clobbering, and leaves the source untouched; for moves it can link/unlink when requested by other callers. | @@ -583,8 +584,10 @@ flag. ### Droid -Droid stores sessions under `~/.factory/sessions//*.jsonl`. The -first row must be `session_start`; ADE reads id, cwd, and title there. Current +Droid stores sessions under `/sessions//*.jsonl` +— `~/.factory` unless `FACTORY_HOME_OVERRIDE` is set, in which case that variable +replaces the HOME `.factory` is appended to rather than naming the directory +itself (`shared/providerConfigHomes.ts`). The first row must be `session_start`; ADE reads id, cwd, and title there. Current Droid rows can omit a start timestamp, so creation time falls back to the first timestamped message. `"New Session"` is a placeholder title and becomes null. The `.settings.json` sidecar holds model/mode, and `sessions-index.json` is diff --git a/docs/features/terminals-and-sessions/pty-and-sessions.md b/docs/features/terminals-and-sessions/pty-and-sessions.md index f0f2842aa..962cc8aef 100644 --- a/docs/features/terminals-and-sessions/pty-and-sessions.md +++ b/docs/features/terminals-and-sessions/pty-and-sessions.md @@ -700,7 +700,17 @@ finalized at close time, and also on demand via `ensureResumeTargets(sessionIds)`. `backfillResumeTargetFromTranscriptBestEffort` is the fire-and-forget wrapper used by close/dispose paths; the on-demand call path is `async` and returns whether a target was -resolved. Strategies, in order: +resolved. + +Every provider-storage path below is resolved through +`services/shared/providerConfigHomes.ts`, never a hardcoded `~/.codex` or +`~/.factory`: `CODEX_HOME` and `CLAUDE_CONFIG_DIR` name the config directory, +while `FACTORY_HOME_OVERRIDE` replaces the HOME that `.factory` is appended to. +Hardcoding the default makes recovery read a different directory than the CLI +ADE just spawned, so a user with an override would silently never recover a +resume target. + +Strategies, in order: 1. Scan the transcript tail with provider-specific regexes (`extractResumeCommandFromOutput`). The regex now matches resume / @@ -715,7 +725,7 @@ resolved. Strategies, in order: (`CLAUDE_STORAGE_MATCH_START_SKEW_MS = 1 s`, `CLAUDE_STORAGE_MATCH_END_SKEW_MS = 5 s`). 3. Read Codex's rollout storage: - `~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl`. The scan now covers + `/sessions/YYYY/MM/DD/rollout-*.jsonl`. The scan now covers up to 7 days of dated directories and up to 80 candidate files. Each candidate's first JSONL line is parsed; sessions whose `session_meta.payload.cwd` matches are scored by closeness between @@ -734,7 +744,7 @@ resolved. Strategies, in order: only enforces a 10-minute drift window so it can match older sessions on resume. 4. Read Droid's local storage: - `~/.factory/sessions//*.jsonl`. Each candidate's first + `/sessions//*.jsonl`. Each candidate's first line must be a `session_start` record whose `cwd` matches the ADE session; the file's mtime is scored against `startedAt` with a 10-minute drift window. The recovered session UUID becomes @@ -763,7 +773,7 @@ never throws. The Codex storage scan is gated by the `reason` argument that the backfill runs under: `"close"` and `"dispose"` consult -`~/.codex/sessions` with the 10-minute drift window; `"session-list"` +`/sessions` with the 10-minute drift window; `"session-list"` skips the storage lookup entirely; `"resume-launch"` is allowed to use the storage lookup because the user is actively trying to continue that specific session. Lazy hydration over `sessions.list` therefore relies @@ -791,7 +801,7 @@ UUID since codex has no pre-assigned-id flag (unlike Claude's - **`fs.watch` on the day directory.** A fresh codex run almost always writes its rollout JSONL within ~1 s. The service watches today's - `~/.codex/sessions/YYYY/MM/DD/` (and tomorrow's, to handle UTC + `/sessions/YYYY/MM/DD/` (and tomorrow's, to handle UTC rollover near midnight); each `add`/`change` event triggers a 200 ms-debounced parse pass against any new candidate file matching the identification rules below. @@ -829,7 +839,7 @@ never captured live. When a UUID is captured, the service writes the row's `resumeMetadata.targetId` and **registers a stable thread name in codex's index**: it appends `{ id, thread_name, updated_at }` to -`~/.codex/session_index.jsonl` with a derived `ade-` +`/session_index.jsonl` with a derived `ade-` name. This is codex's public on-disk format for `SetThreadName`, so once the line lands, `codex resume ade-` resolves through the index regardless of where the rollout file ends up on disk. We diff --git a/docs/features/terminals-and-sessions/runtime-isolation.md b/docs/features/terminals-and-sessions/runtime-isolation.md index 6606d9a67..a220fc985 100644 --- a/docs/features/terminals-and-sessions/runtime-isolation.md +++ b/docs/features/terminals-and-sessions/runtime-isolation.md @@ -127,10 +127,13 @@ inside the lane worktree, but hostname-based isolation is off. ## What isolation does NOT cover -- **Shared filesystem locations** — `~/.claude/`, `~/.codex/`, global - npm/yarn/pip caches, host-level docker daemon. The PTY inherits - `process.env` including `HOME`, so CLIs write to their usual user - paths regardless of the lane. +- **Shared filesystem locations** — `~/.claude/`, `~/.codex/`, + `~/.factory/`, global npm/yarn/pip caches, host-level docker daemon. The + PTY inherits `process.env` including `HOME`, so CLIs write to their usual + user paths regardless of the lane — and their usual paths are whatever + `CLAUDE_CONFIG_DIR` / `CODEX_HOME` / `FACTORY_HOME_OVERRIDE` say, which is + why every ADE read of those directories goes through + `services/shared/providerConfigHomes.ts` instead of a hardcoded default. - **Network sockets** — lane port ranges are advisory; commands can bind to any free port unless explicitly constrained. - **Shared database** — ADE's own SQLite file is per-project, not per lane. All lanes in a project write into the same `terminal_sessions` From 07b88b6e06c859c58f7dcc6fd1325ac146de99e3 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 06:33:28 -0400 Subject: [PATCH 15/18] review: fix four findings from Codex, Cursor Bugbot and CodeRabbit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bots independently found that the Spec-mode escape flag only covered one of the three ways a session can enter Spec. `sessionOptions` can start the SDK directly in Spec, and the resume-failure fallback creates a session the same way, so a session created in Spec had no recorded way back out — and the SDK has no exitSpecMode. Both paths now record it. The cross-worker case is documented rather than papered over: a session a PREVIOUS worker left in Spec cannot be detected, because that state lives in Droid and the SDK exposes no way to read it back, and assuming Spec on every resume would state a mode ADE does not own. Cursor Bugbot and CodeRabbit both caught that listing `/output-style` with no argument wrote the resolved name onto the session and persisted it. Every later option build then treated that cache as a real selection, so ADE sent outputStyle at flag tier and suppressed Claude's own resolution — reintroducing the exact override this branch removes, through the display path. The listing no longer mutates the session, and reads the settings files before the cache so a newer selection wins. Codex caught that the creation path still substituted "auto-low" for a Droid session that requested no mode, which is why the resolver could never return null. That is the same shape as the `?? "Default"` bug this branch fixes: the substituted value is persisted and read back as a real choice. The desktop composer always sends a mode, so this only changes launches that send nothing — which is precisely the case that should inherit. Codex also caught a regression in the local-provider trim: ollama is not in OpenCode's catalog, so when discovery found models but the user had typed no endpoint, the models were named with no address to reach them — worst for an isolated lead, which inherits no user config at all. A user-typed endpoint still wins; the default only fills the gap. Regression tests added for the output-style listing and both ollama cases; each was verified to fail without its fix. The listing test lives in its own suite because adding it inside the existing block perturbed shared state two Cursor recovery tests depend on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../services/chat/agentChatService.test.ts | 20 +++++++++++++++++++ .../main/services/chat/agentChatService.ts | 17 +++++++++++----- .../src/main/services/chat/droidSdkWorker.ts | 14 ++++++++++++- .../services/opencode/openCodeRuntime.test.ts | 20 +++++++++++++++++++ .../main/services/opencode/openCodeRuntime.ts | 15 +++++++++++--- 5 files changed, 77 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index a0573b059..dd4a227d5 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -42375,3 +42375,23 @@ describe("host sleep narration", () => { } }); }); + +describe("claude output style listing", () => { + it("does not persist an output style just because the list was shown", async () => { + // Listing styles used to write the resolved name onto the session and + // persist it. Every later option build then treated that cache as a real + // selection, so ADE sent outputStyle at flag tier and suppressed Claude's + // own resolution — the override this branch exists to stop. + const { service } = createService(); + const session = await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "claude-sonnet-5", + }); + + await service.sendMessage({ sessionId: session.id, text: "/output-style" }); + + expect(readPersistedChatState(session.id).claudeOutputStyle ?? null).toBeNull(); + expect((await service.getSessionSummary(session.id))?.claudeOutputStyle ?? null).toBeNull(); + }); +}); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 5d583a465..2676675f7 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -31160,10 +31160,13 @@ export function createAgentChatService(args: { interactionMode: effectiveInteractionMode === "plan" || effectivePermissionMode === "plan" ? "plan" as const : "default" as const, + // No fallback: a substituted mode here is persisted and then read back + // as a real selection, which is what kept the inheritance path dead. + // The desktop composer always sends one; a launch that sends nothing + // is saying nothing, and Droid resolves it from settings.json. droidPermissionMode: requestedDroidPermissionMode ?? legacyPermissionModeToDroidPermissionMode(effectivePermissionMode) - ?? legacyOpenCodePermissionModeToDroidPermissionMode(requestedOpenCodePermissionMode) - ?? "auto-low", + ?? legacyOpenCodePermissionModeToDroidPermissionMode(requestedOpenCodePermissionMode), }; } if (effectiveProvider === "pi") { @@ -46041,15 +46044,19 @@ export function createAgentChatService(args: { const requestedStyle = match[1]?.trim() ?? ""; managed.session.lastActivityAt = nowIso(); if (!requestedStyle.length) { - managed.session.claudeOutputStyle = managed.session.claudeOutputStyle - ?? readClaudeOutputStyleSelection(managed.laneWorktreePath) + // Display only. Writing the resolved name back would persist "Default" + // as though the user had picked it, and ADE would then send it at flag + // tier and suppress Claude's own resolution — the override this branch + // exists to stop. Read the files first so a newer selection wins. + const listedStyle = readClaudeOutputStyleSelection(managed.laneWorktreePath) + ?? normalizePersistedOutputStyle(managed.session.claudeOutputStyle) ?? "Default"; emitChatEvent(managed, { type: "system_notice", noticeKind: "info", message: renderClaudeOutputStyleList( discoverClaudeOutputStyles(managed.laneWorktreePath), - managed.session.claudeOutputStyle, + listedStyle, ), }); persistChatState(managed); diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index f513788a5..5dd7f614e 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -24,7 +24,17 @@ type DroidSession = Awaited>; let sdkModule: DroidSdkModule | null = null; let initState: DroidSdkWorkerInit | null = null; let session: DroidSession | null = null; -// Set when ADE itself put this session into Spec mode; see applySettings. +/** + * Set when ADE put THIS session into Spec mode — on create, on the + * resume-failure fallback, or in applySettings. It is the only way back out, + * because the SDK has no exitSpecMode. + * + * Known limit: a session resumed into a fresh worker that a PREVIOUS worker had + * put into Spec starts with the flag false, and the SDK exposes no way to read + * the live mode back. Reaching that case needs a plan session, a worker + * restart, plan turned off, and no chosen permission mode — and the alternative + * (assuming Spec on every resume) would state a mode ADE does not own. + */ let enteredSpecMode = false; const activeAborts = new Set(); let waiterSeq = 0; @@ -366,9 +376,11 @@ async function initWorker(init: DroidSdkWorkerInit): Promise { detail: { resumeSessionId: resumeId, error: errorMessage(error) }, }); session = await sdk.createSession(sessionOptions(sdk, init, init.settings)); + enteredSpecMode = init.settings.interactionMode === "spec"; } } else { session = await sdk.createSession(sessionOptions(sdk, init, init.settings)); + enteredSpecMode = init.settings.interactionMode === "spec"; } // `createSession`/`resumeSession` take `disabledToolIds`, but the ids are // only discoverable from the live session, so the lead's denial is pushed diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts index 51e854265..bde38a1d9 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts @@ -403,6 +403,26 @@ describe("buildOpenCodeConfig user-owned keys", () => { } }); + it("gives discovered ollama models an endpoint to run against", () => { + // Naming the models without an address leaves them unrunnable, and ollama is + // not in OpenCode's catalog so nothing else can supply one — an isolated + // lead inherits no user config at all. + const cfg = buildOpenCodeConfig({ + projectConfig: { ai: {} } as any, + discoveredLocalModels: [{ provider: "ollama", modelId: "llama3", loaded: true }], + } as any) as Record; + expect(cfg.provider.ollama.models).toHaveProperty("llama3"); + expect(cfg.provider.ollama.options.baseURL).toBeTruthy(); + }); + + it("keeps a user-configured ollama endpoint over ADE's default", () => { + const cfg = buildOpenCodeConfig({ + projectConfig: { ai: { localProviders: { ollama: { endpoint: "http://remote-box:11434" } } } } as any, + discoveredLocalModels: [{ provider: "ollama", modelId: "llama3", loaded: true }], + } as any) as Record; + expect(cfg.provider.ollama.options.baseURL).toContain("remote-box"); + }); + it("omits local providers the user never configured", () => { // An ADE-invented baseURL merges over the endpoint in the user's own // opencode.json, repointing a configured remote host back at localhost. diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts index 8142a26dc..b8848df19 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts @@ -14,6 +14,7 @@ import { type OpencodeClient as OpenCodeV2Client, } from "@opencode-ai/sdk/v2/client"; import { + getLocalProviderDefaultEndpoint, decodeOpenCodeRegistryId, ensureOpenCodeBaseURL, type LocalProviderFamily, @@ -387,18 +388,26 @@ function buildProviderConfig( } } const endpoint = trimToUndefined(settings?.endpoint); + const discoveredModelCount = Object.keys(models).length; // Say nothing about a provider the user never set up. This config is merged // last and per key, so an ADE-invented baseURL would overwrite the endpoint // in the user's own opencode.json — repointing a configured remote host back // at localhost. Only an endpoint the user actually typed, or models ADE // discovered, justify naming the provider at all. - if (!endpoint && !Object.keys(models).length) return; + if (!endpoint && !discoveredModelCount) return; + // ollama is not in OpenCode's provider catalog, so nothing else can supply + // its package or address. If ADE discovered models it reached them at some + // endpoint, and naming the models without one leaves them unrunnable — + // especially for an isolated lead, which inherits no user config at all. + // A user-typed endpoint always wins; this only fills the gap. + const resolvedEndpoint = endpoint + ?? (family === "ollama" && discoveredModelCount ? getLocalProviderDefaultEndpoint(family) : undefined); provider[family] = { // lmstudio ships in OpenCode's provider catalog with its own npm package // and baseURL; ollama does not, so only ollama needs one stated here. ...(family === "ollama" ? { npm: "@ai-sdk/openai-compatible" } : {}), - ...(endpoint ? { options: { baseURL: ensureOpenCodeBaseURL(endpoint) } } : {}), - ...(Object.keys(models).length > 0 ? { models } : {}), + ...(resolvedEndpoint ? { options: { baseURL: ensureOpenCodeBaseURL(resolvedEndpoint) } } : {}), + ...(discoveredModelCount > 0 ? { models } : {}), }; }; From 54cc54bcbb414d1836acb715e7c80a06181369fb Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 06:50:04 -0400 Subject: [PATCH 16/18] droid: read the resumed session's mode instead of assuming it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's remaining case was real and reachable: turn plan off while a Droid chat is idle and its worker has been evicted, and the next message resumes a session Droid still has in Spec while ADE now states nothing — leaving it read-only with no way out, because the SDK has no exitSpecMode. ADE cannot remember this on its own; the state lives in Droid. But the resumed session hands its resolved settings back in initResult, so the worker can simply read the live interactionMode and seed the escape flag from it. That is strictly better than the alternative of assuming Spec on every resume, which would have meant stating a mode ADE does not own — overriding a user who configured interactionMode in their own settings.json. Two Codex P1s on this push were re-anchored copies of findings already fixed in 07b88b6e0 (the "auto-low" creation fallback is gone; the Spec flag is set on both create paths). Verified against the current code rather than re-fixed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../src/main/services/chat/droidSdkWorker.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index 5dd7f614e..2e6691680 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -244,6 +244,22 @@ function normalizeAvailableModels(initResult: unknown): DroidSdkReady["available * `initResult.currentModelId` does not exist — @factory/droid-sdk reports the * resolved settings under `initResult.settings`. */ +/** + * The interaction mode Droid reports for the session it just handed back. + * + * On resume this is the only way to learn that a PREVIOUS worker left the + * session in Spec: that state lives in Droid, not in ADE, and the SDK has no + * exitSpecMode. Reading it back beats assuming, which would have meant stating + * a mode ADE does not own on every resume. + */ +function readResolvedInteractionMode(initResult: unknown): string | null { + const record = initResult && typeof initResult === "object" ? initResult as Record : null; + const settings = record?.settings && typeof record.settings === "object" + ? record.settings as Record + : null; + return typeof settings?.interactionMode === "string" ? settings.interactionMode : null; +} + function readResolvedModelId(initResult: unknown): string | null { const record = initResult && typeof initResult === "object" ? initResult as Record : null; const settings = record?.settings && typeof record.settings === "object" @@ -367,6 +383,9 @@ async function initWorker(init: DroidSdkWorkerInit): Promise { askUserHandler: requestAskUser, ...(init.mcpServers?.length ? { mcpServers: init.mcpServers as DroidSdkTypes.ResumeSessionOptions["mcpServers"] } : {}), }); + // Seed from what Droid reports before applying, so a session a previous + // worker left in Spec still has a way out even when ADE now states nothing. + enteredSpecMode = readResolvedInteractionMode(session.initResult) === "spec"; await applySettings(init.settings); } catch (error) { post({ From 1b72bfb991e9a7efa002079c1ae7ec45900e76b6 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:11:32 -0400 Subject: [PATCH 17/18] pty: honour CLAUDE_CONFIG_DIR for Claude session storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught a genuine miss in my own sweep: this file was edited for exactly this bug class, but only the Codex and Factory resolvers were replaced. claudeProjectDirForCwd still built `/.claude/projects`, so with CLAUDE_CONFIG_DIR set, Claude storage backfill and runtime-title capture read a directory the CLI is not writing to. The fixtures in ptyService.test.ts now resolve through the same helper the production path uses. The shared test setup already points CLAUDE_CONFIG_DIR at a temp directory, so those cases only pass while the code honours it — verified by reverting the fix, which fails three of them. That is the regression coverage the review asked for, without a new test file. All five provider-config call sites in this file now pass os.homedir() explicitly, so the module's own node:os mock governs the fallback rather than the shared helper's named import. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../src/main/services/pty/ptyService.test.ts | 13 ++++++------- apps/desktop/src/main/services/pty/ptyService.ts | 12 ++++++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/main/services/pty/ptyService.test.ts b/apps/desktop/src/main/services/pty/ptyService.test.ts index 7a6d39c17..489178923 100644 --- a/apps/desktop/src/main/services/pty/ptyService.test.ts +++ b/apps/desktop/src/main/services/pty/ptyService.test.ts @@ -354,6 +354,7 @@ import { selectPiStorageSessionCandidate, } from "./ptyService"; import { resolveBuiltInBrowserActorCapability } from "../builtInBrowser/builtInBrowserActorCapabilities"; +import { claudeConfigHome } from "../shared/providerConfigHomes"; const originalPlatform = process.platform; const originalHome = process.env.HOME; @@ -6001,8 +6002,7 @@ describe("ptyService", () => { try { const claudeSessionId = "123e4567-e89b-12d3-a456-426614174000"; const claudeFilePath = path.join( - os.homedir(), - ".claude", + claudeConfigHome({ homeDir: os.homedir() }), "projects", "-tmp-test-worktree", `${claudeSessionId}.jsonl`, @@ -8087,7 +8087,7 @@ describe("ptyService", () => { const matchedId = "11111111-1111-1111-1111-111111111111"; const newerDifferentId = "22222222-2222-2222-2222-222222222222"; - const claudeProjectDir = path.join(os.homedir(), ".claude", "projects", "-tmp-test-worktree"); + const claudeProjectDir = path.join(claudeConfigHome({ homeDir: os.homedir() }), "projects", "-tmp-test-worktree"); const matchedPath = path.join(claudeProjectDir, `${matchedId}.jsonl`); const newerDifferentPath = path.join(claudeProjectDir, `${newerDifferentId}.jsonl`); const matchedFirstLine = JSON.stringify({ @@ -8149,7 +8149,7 @@ describe("ptyService", () => { vi.setSystemTime(fakeNow); const otherId = "33333333-3333-3333-3333-333333333333"; - const claudeProjectDir = path.join(os.homedir(), ".claude", "projects", "-tmp-test-worktree"); + const claudeProjectDir = path.join(claudeConfigHome({ homeDir: os.homedir() }), "projects", "-tmp-test-worktree"); const otherPath = path.join(claudeProjectDir, `${otherId}.jsonl`); const otherFirstLine = JSON.stringify({ timestamp: "2026-04-15T21:31:00.000Z", @@ -8202,7 +8202,7 @@ describe("ptyService", () => { const firstId = "44444444-4444-4444-4444-444444444444"; const secondId = "55555555-5555-5555-5555-555555555555"; - const claudeProjectDir = path.join(os.homedir(), ".claude", "projects", "-tmp-test-worktree"); + const claudeProjectDir = path.join(claudeConfigHome({ homeDir: os.homedir() }), "projects", "-tmp-test-worktree"); const firstPath = path.join(claudeProjectDir, `${firstId}.jsonl`); const secondPath = path.join(claudeProjectDir, `${secondId}.jsonl`); const firstLine = JSON.stringify({ @@ -8422,8 +8422,7 @@ describe("ptyService", () => { try { const claudeSessionId = "5647da1e-10de-4089-bce2-00b9c2552bfc"; const filePath = path.join( - os.homedir(), - ".claude", + claudeConfigHome({ homeDir: os.homedir() }), "projects", "-tmp-test-worktree", `${claudeSessionId}.jsonl`, diff --git a/apps/desktop/src/main/services/pty/ptyService.ts b/apps/desktop/src/main/services/pty/ptyService.ts index 598264ce6..fc767c9df 100644 --- a/apps/desktop/src/main/services/pty/ptyService.ts +++ b/apps/desktop/src/main/services/pty/ptyService.ts @@ -135,7 +135,7 @@ import { claudeAgentSkillPluginRoots } from "../skills/agentSkillRuntimeService" import { stripAnsi } from "../../utils/ansiStrip"; import { summarizeTerminalSession } from "../../utils/sessionSummary"; import { derivePreviewFromChunk, type PreviewCursorState } from "../../utils/terminalPreview"; -import { codexConfigHome, factoryConfigHome } from "../shared/providerConfigHomes"; +import { claudeConfigHome, codexConfigHome, factoryConfigHome } from "../shared/providerConfigHomes"; import { clearTuiWaitingInput, createTuiMarkerState, @@ -2957,7 +2957,7 @@ export function createPtyService({ // every non-alphanumeric character into `-` without collapsing runs or // trimming. Cursor and Droid each escape differently; reuse the one that // already encodes Claude's rule rather than generalise across vendors. - return path.join(os.homedir(), ".claude", "projects", claudeProjectSlugForCwd(cwd)); + return path.join(claudeConfigHome({ homeDir: os.homedir() }), "projects", claudeProjectSlugForCwd(cwd)); } function claudeSessionFilePathForCwd(cwd: string, claudeSessionId: string): string { @@ -3208,7 +3208,7 @@ export function createPtyService({ } function readCodexThreadNameFromIndex(codexSessionId: string): string | null { - const indexPath = path.join(codexConfigHome(), "session_index.jsonl"); + const indexPath = path.join(codexConfigHome({ homeDir: os.homedir() }), "session_index.jsonl"); const text = readFileSuffix(indexPath, CODEX_THREAD_NAME_SCAN_BYTES); if (!text) return null; const lines = text.split(/\r?\n/).filter(Boolean); @@ -3250,7 +3250,7 @@ export function createPtyService({ ownershipOriginator?: string | null; }): CodexStorageSessionMatch | null => { try { - const sessionsBase = path.join(codexConfigHome(), "sessions"); + const sessionsBase = path.join(codexConfigHome({ homeDir: os.homedir() }), "sessions"); if (!fs.existsSync(sessionsBase)) return null; const now = new Date(); @@ -3385,7 +3385,7 @@ export function createPtyService({ maxStartDeltaMs?: number; }): string | null => { try { - const droidSessionsDir = path.join(factoryConfigHome(), "sessions"); + const droidSessionsDir = path.join(factoryConfigHome({ homeDir: os.homedir() }), "sessions"); if (!fs.existsSync(droidSessionsDir)) return null; const projectEntries = fs.readdirSync(droidSessionsDir, { withFileTypes: true }) .filter((entry) => entry.isDirectory()); @@ -3945,7 +3945,7 @@ export function createPtyService({ ): void => { const startedAtMs = Date.parse(startedAt); const startedAtFinite = Number.isFinite(startedAtMs) ? startedAtMs : null; - const sessionsBase = path.join(codexConfigHome(), "sessions"); + const sessionsBase = path.join(codexConfigHome({ homeDir: os.homedir() }), "sessions"); let captured = false; const watchers: Array<{ close: () => void }> = []; const timers = new Set(); From 98ad54cae104d423dc01e41baa0dfdfc97a3dda1 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 20 Aug 2026 07:34:00 -0400 Subject: [PATCH 18/18] review: don't infer Spec ownership, and don't invent an endpoint over user config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from CodeRabbit, both of which caught my previous fix trading one override for another. Reading the resumed session's interactionMode cannot tell a Spec that ADE entered from one the user configured in ~/.factory/settings.json. Exiting the latter would be precisely the override this branch removes, so the inference is gone. applySettings still records Spec whenever ADE itself states it, which covers every resume ADE drives; the residual — a session left in Spec by a previous worker while ADE now states nothing — stays documented rather than "fixed" by overriding a user setting. CodeRabbit suggested this inference in the prior round and then flagged it here; the flag is right. The ollama endpoint has the same shape. OPENCODE_CONFIG_CONTENT merges last, so an ADE default can replace a remote host in the user's own opencode.json, which ADE cannot read. But an isolated lead inherits no user config at all, so there is nothing to clobber and nothing else to supply the address — which is the case Codex's original finding was actually about. The fallback is now gated on isolatedConfig, so a lead's discovered models stay runnable and an ordinary session keeps deferring to the user's own file. Both directions are pinned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nv4uczqGgnQBnrz1tNz5Lj --- .../src/main/services/chat/droidSdkWorker.ts | 24 ++++--------------- .../services/opencode/openCodeRuntime.test.ts | 23 +++++++++++++----- .../main/services/opencode/openCodeRuntime.ts | 18 +++++++++----- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/apps/desktop/src/main/services/chat/droidSdkWorker.ts b/apps/desktop/src/main/services/chat/droidSdkWorker.ts index 2e6691680..922cb652f 100644 --- a/apps/desktop/src/main/services/chat/droidSdkWorker.ts +++ b/apps/desktop/src/main/services/chat/droidSdkWorker.ts @@ -244,22 +244,6 @@ function normalizeAvailableModels(initResult: unknown): DroidSdkReady["available * `initResult.currentModelId` does not exist — @factory/droid-sdk reports the * resolved settings under `initResult.settings`. */ -/** - * The interaction mode Droid reports for the session it just handed back. - * - * On resume this is the only way to learn that a PREVIOUS worker left the - * session in Spec: that state lives in Droid, not in ADE, and the SDK has no - * exitSpecMode. Reading it back beats assuming, which would have meant stating - * a mode ADE does not own on every resume. - */ -function readResolvedInteractionMode(initResult: unknown): string | null { - const record = initResult && typeof initResult === "object" ? initResult as Record : null; - const settings = record?.settings && typeof record.settings === "object" - ? record.settings as Record - : null; - return typeof settings?.interactionMode === "string" ? settings.interactionMode : null; -} - function readResolvedModelId(initResult: unknown): string | null { const record = initResult && typeof initResult === "object" ? initResult as Record : null; const settings = record?.settings && typeof record.settings === "object" @@ -383,9 +367,11 @@ async function initWorker(init: DroidSdkWorkerInit): Promise { askUserHandler: requestAskUser, ...(init.mcpServers?.length ? { mcpServers: init.mcpServers as DroidSdkTypes.ResumeSessionOptions["mcpServers"] } : {}), }); - // Seed from what Droid reports before applying, so a session a previous - // worker left in Spec still has a way out even when ADE now states nothing. - enteredSpecMode = readResolvedInteractionMode(session.initResult) === "spec"; + // Deliberately NOT seeded from the mode Droid reports here. That reading + // cannot tell a Spec this ADE entered from one the user configured in + // ~/.factory/settings.json, and exiting the latter would be exactly the + // override this branch removes. applySettings below sets the flag when ADE + // itself restates Spec, which covers every resume ADE drives. await applySettings(init.settings); } catch (error) { post({ diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts index bde38a1d9..60d71441a 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.test.ts @@ -403,16 +403,27 @@ describe("buildOpenCodeConfig user-owned keys", () => { } }); - it("gives discovered ollama models an endpoint to run against", () => { - // Naming the models without an address leaves them unrunnable, and ollama is - // not in OpenCode's catalog so nothing else can supply one — an isolated - // lead inherits no user config at all. + it("gives an isolated lead's discovered ollama models an endpoint to run against", () => { + // A lead inherits no user config, so nothing else can supply the address and + // there is no user endpoint to clobber. + const cfg = buildOpenCodeConfig({ + projectConfig: { ai: {} } as any, + isolatedConfig: true, + discoveredLocalModels: [{ provider: "ollama", modelId: "llama3", loaded: true }], + } as any) as Record; + expect(cfg.provider.ollama.models).toHaveProperty("llama3"); + expect(cfg.provider.ollama.options.baseURL).toBe("http://localhost:11434/v1"); + }); + + it("does not invent an ollama endpoint for an ordinary session", () => { + // OPENCODE_CONFIG_CONTENT merges last, so an ADE default would replace a + // remote host in the user's own opencode.json — which ADE cannot read. const cfg = buildOpenCodeConfig({ projectConfig: { ai: {} } as any, discoveredLocalModels: [{ provider: "ollama", modelId: "llama3", loaded: true }], } as any) as Record; expect(cfg.provider.ollama.models).toHaveProperty("llama3"); - expect(cfg.provider.ollama.options.baseURL).toBeTruthy(); + expect(cfg.provider.ollama.options?.baseURL).toBeUndefined(); }); it("keeps a user-configured ollama endpoint over ADE's default", () => { @@ -420,7 +431,7 @@ describe("buildOpenCodeConfig user-owned keys", () => { projectConfig: { ai: { localProviders: { ollama: { endpoint: "http://remote-box:11434" } } } } as any, discoveredLocalModels: [{ provider: "ollama", modelId: "llama3", loaded: true }], } as any) as Record; - expect(cfg.provider.ollama.options.baseURL).toContain("remote-box"); + expect(cfg.provider.ollama.options.baseURL).toBe("http://remote-box:11434/v1"); }); it("omits local providers the user never configured", () => { diff --git a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts index b8848df19..428a89b8b 100644 --- a/apps/desktop/src/main/services/opencode/openCodeRuntime.ts +++ b/apps/desktop/src/main/services/opencode/openCodeRuntime.ts @@ -120,6 +120,8 @@ type BuildOpenCodeConfigArgs = { /** Dynamically discovered models from local provider endpoints (e.g. LM Studio /v1/models). */ discoveredLocalModels?: DiscoveredLocalModelEntry[]; mcp?: OpenCodeConfig["mcp"]; + /** Lead servers inherit no user config, so ADE must supply what they need. */ + isolatedConfig?: boolean; }; type StartOpenCodeSessionArgs = BuildOpenCodeConfigArgs & { @@ -317,6 +319,7 @@ const KNOWN_OPENCODE_CATALOG_PROVIDER_IDS: ReadonlySet = new Set([ function buildProviderConfig( projectConfig: ProjectConfigFile | EffectiveProjectConfig, discoveredLocalModels?: DiscoveredLocalModelEntry[], + isolatedConfig?: boolean, ): OpenCodeConfig["provider"] | undefined { const ai = projectConfig.ai ?? {}; const apiKeys = ai.apiKeys ?? {}; @@ -396,12 +399,15 @@ function buildProviderConfig( // discovered, justify naming the provider at all. if (!endpoint && !discoveredModelCount) return; // ollama is not in OpenCode's provider catalog, so nothing else can supply - // its package or address. If ADE discovered models it reached them at some - // endpoint, and naming the models without one leaves them unrunnable — - // especially for an isolated lead, which inherits no user config at all. - // A user-typed endpoint always wins; this only fills the gap. + // its address, and naming models without one leaves them unrunnable. But + // OPENCODE_CONFIG_CONTENT merges last, so an ADE default would replace a + // remote endpoint in the user's own opencode.json — which ADE cannot read. + // Only an isolated lead is safe to fill in: it inherits no user config, so + // there is nothing to clobber and nothing else to supply the address. const resolvedEndpoint = endpoint - ?? (family === "ollama" && discoveredModelCount ? getLocalProviderDefaultEndpoint(family) : undefined); + ?? (isolatedConfig && family === "ollama" && discoveredModelCount + ? getLocalProviderDefaultEndpoint(family) + : undefined); provider[family] = { // lmstudio ships in OpenCode's provider catalog with its own npm package // and baseURL; ollama does not, so only ollama needs one stated here. @@ -507,7 +513,7 @@ function mergeCustomModelSlugs( } export function buildOpenCodeConfig(args: BuildOpenCodeConfigArgs): OpenCodeConfig { - const provider = buildProviderConfig(args.projectConfig, args.discoveredLocalModels); + const provider = buildProviderConfig(args.projectConfig, args.discoveredLocalModels, args.isolatedConfig); const helperPermission = { edit: "deny", bash: "deny",