From 35dfc4029d3ef58c2d32c5c457e44d895fe9aaa7 Mon Sep 17 00:00:00 2001 From: Rui Rei Date: Fri, 5 Jun 2026 14:38:51 +0100 Subject: [PATCH 1/7] fix: use bare peerName for the user peer to match sibling plugins The user peer was derived as `user:${peerName}` (normalised to `user-`), whereas the claude-honcho and hermes-honcho plugins use the bare `peerName`. In a shared workspace this split the user's memory into a separate `user-` collection, so OpenCode never saw or contributed to what the other tools learned. Drop the prefix so the user peer matches the sibling plugins. --- src/index.ts | 6 +++++- tests/context-injection.test.js | 2 +- tests/peer-topology.test.js | 8 ++++---- tests/user-peer-id.test.js | 15 +++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) create mode 100644 tests/user-peer-id.test.js diff --git a/src/index.ts b/src/index.ts index f19e669..23a41d8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -616,6 +616,9 @@ const writeSettings = async ( const currentUserName = () => "user" +const deriveUserPeerId = (settings: Pick) => + normalizeId(settings.peerName || currentUserName()) + const rootApiKey = (raw: Record) => { const legacyApiKey = typeof raw[LEGACY_API_KEY_FIELD] === "string" ? expandEnv(raw[LEGACY_API_KEY_FIELD] as string) : "" return legacyApiKey @@ -754,7 +757,7 @@ const deriveRuntimeHandle = async ( const sessionId = extractSessionId(input) const repoName = path.basename(rootDir) const workspaceId = normalizeId(settings.workspace || "opencode") - const userPeerId = normalizeId(`user:${settings.peerName || currentUserName()}`) + const userPeerId = deriveUserPeerId(settings) const rootAgentPeerId = normalizeId(settings.aiPeer || "opencode") const activeAgentPeerId = rootAgentPeerId const childAgentPeerId = null @@ -1667,6 +1670,7 @@ export const createHonchoRuntimePlugin = export const HonchoRuntimePlugin = createHonchoRuntimePlugin() export const __testing = { createSessionState, + deriveUserPeerId, deriveSessionStateKey, extractCompletedAssistantMessage, honchoSdkImportPath: "@honcho-ai/sdk", diff --git a/tests/context-injection.test.js b/tests/context-injection.test.js index c2712ae..0c5e9d6 100644 --- a/tests/context-injection.test.js +++ b/tests/context-injection.test.js @@ -96,7 +96,7 @@ const createHonchoFetch = ({ failStableHydration = false } = {}) => { return jsonResponse({ peer_id: peerId, target_id: null, - representation: peerId.startsWith("user-") + representation: peerId === "user" ? "The user prefers concise engineering analysis." : "The assistant is working on opencode-honcho.", peer_card: ["Keep changes narrowly scoped."], diff --git a/tests/peer-topology.test.js b/tests/peer-topology.test.js index 02167d8..66abe01 100644 --- a/tests/peer-topology.test.js +++ b/tests/peer-topology.test.js @@ -5,7 +5,7 @@ import { __testing } from "../dist/index.js" test("root sessions keep user and root agent as peers", () => { const topology = __testing.buildPeerTopology({ config: {}, - userPeerId: "user:user", + userPeerId: "user", rootAgentPeerId: "opencode", activeAgentPeerId: "opencode", childAgentPeerId: null, @@ -13,7 +13,7 @@ test("root sessions keep user and root agent as peers", () => { }) expect(topology.sessionPeerConfigs).toEqual({ - "user:user": { observeMe: true, observeOthers: false }, + "user": { observeMe: true, observeOthers: false }, opencode: { observeMe: true, observeOthers: true }, }) expect(topology.describedPeers.childAgentPeer).toBeNull() @@ -23,7 +23,7 @@ test("root sessions keep user and root agent as peers", () => { test("classic peer model keeps delegated sessions on the Claude-style user and ai peers", () => { const topology = __testing.buildPeerTopology({ config: {}, - userPeerId: "user:user", + userPeerId: "user", rootAgentPeerId: "opencode", activeAgentPeerId: "opencode:reviewer", childAgentPeerId: "opencode:reviewer", @@ -31,7 +31,7 @@ test("classic peer model keeps delegated sessions on the Claude-style user and a }) expect(topology.sessionPeerConfigs).toEqual({ - "user:user": { observeMe: true, observeOthers: false }, + "user": { observeMe: true, observeOthers: false }, opencode: { observeMe: true, observeOthers: true }, }) expect(topology.describedPeers.childAgentPeer).toBeNull() diff --git a/tests/user-peer-id.test.js b/tests/user-peer-id.test.js new file mode 100644 index 0000000..fb29f6c --- /dev/null +++ b/tests/user-peer-id.test.js @@ -0,0 +1,15 @@ +import { expect, test } from "bun:test" + +import { __testing } from "../dist/index.js" + +test("user peer id is the bare peerName with no prefix", () => { + expect(__testing.deriveUserPeerId({ peerName: "rui" })).toBe("rui") +}) + +test("user peer id falls back to the current user name when peerName is empty", () => { + expect(__testing.deriveUserPeerId({ peerName: "" })).toBe("user") +}) + +test("user peer id is normalised", () => { + expect(__testing.deriveUserPeerId({ peerName: "Rui Rei" })).toBe("rui-rei") +}) From 7b190e05c14da2bc7b865769741afa203af6f8df Mon Sep 17 00:00:00 2001 From: Rui Rei Date: Fri, 5 Jun 2026 17:14:01 +0100 Subject: [PATCH 2/7] fix: reject configs where the user and agent peers collide Removing the user: prefix means peerName and aiPeer can now normalise to the same peer id (e.g. both "opencode"), silently merging user and agent memory into one peer. Fail fast on that collision instead. --- src/index.ts | 10 ++++++++++ tests/peer-collision.test.js | 13 +++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/peer-collision.test.js diff --git a/src/index.ts b/src/index.ts index 23a41d8..d2f2684 100644 --- a/src/index.ts +++ b/src/index.ts @@ -619,6 +619,14 @@ const currentUserName = () => "user" const deriveUserPeerId = (settings: Pick) => normalizeId(settings.peerName || currentUserName()) +const assertDistinctUserAndAgentPeers = (userPeerId: string, rootAgentPeerId: string) => { + if (userPeerId === rootAgentPeerId) { + throw new Error( + `Invalid Honcho config: peerName and aiPeer both resolve to the peer id '${userPeerId}'; they must differ so user and agent memory stay separate.`, + ) + } +} + const rootApiKey = (raw: Record) => { const legacyApiKey = typeof raw[LEGACY_API_KEY_FIELD] === "string" ? expandEnv(raw[LEGACY_API_KEY_FIELD] as string) : "" return legacyApiKey @@ -759,6 +767,7 @@ const deriveRuntimeHandle = async ( const workspaceId = normalizeId(settings.workspace || "opencode") const userPeerId = deriveUserPeerId(settings) const rootAgentPeerId = normalizeId(settings.aiPeer || "opencode") + assertDistinctUserAndAgentPeers(userPeerId, rootAgentPeerId) const activeAgentPeerId = rootAgentPeerId const childAgentPeerId = null const parentAgentObserverPeerId = null @@ -1671,6 +1680,7 @@ export const HonchoRuntimePlugin = createHonchoRuntimePlugin() export const __testing = { createSessionState, deriveUserPeerId, + assertDistinctUserAndAgentPeers, deriveSessionStateKey, extractCompletedAssistantMessage, honchoSdkImportPath: "@honcho-ai/sdk", diff --git a/tests/peer-collision.test.js b/tests/peer-collision.test.js new file mode 100644 index 0000000..6245713 --- /dev/null +++ b/tests/peer-collision.test.js @@ -0,0 +1,13 @@ +import { expect, test } from "bun:test" + +import { __testing } from "../dist/index.js" + +test("distinct user and agent peers are accepted", () => { + expect(() => __testing.assertDistinctUserAndAgentPeers("rui", "opencode")).not.toThrow() +}) + +test("colliding user and agent peers are rejected", () => { + expect(() => __testing.assertDistinctUserAndAgentPeers("opencode", "opencode")).toThrow( + /peerName and aiPeer both resolve to the peer id 'opencode'/, + ) +}) From 3db6d5cb2902416d0b2a051b079caa500073205a Mon Sep 17 00:00:00 2001 From: adavyas Date: Mon, 15 Jun 2026 19:49:22 -0400 Subject: [PATCH 3/7] feat: gate user-peer prefix on hosts.opencode.removeUserPrefix PR #23 dropped the user-peer prefix unconditionally, which re-points every existing user at an empty `` peer and orphans their accumulated memory. Gate it on a boolean under the opencode host block instead: hosts.opencode.removeUserPrefix: - new installs are stamped `true` at config creation (runtime + TUI) -> bare ``, matching the sibling claude-honcho / hermes-honcho plugins - existing/upgraded installs default `false` -> keep `user-` and its memory; the config file is never mutated on upgrade - it's a normal validated host setting, so anyone can opt in via the file, /honcho:set-config, or the TUI Builds on @rrei's #23: keeps the bare derivation and the user/agent peer collision guard; generalizes deriveUserPeerId to honor the flag. Co-authored-by: Rui Rei Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 37 ++++++++++++++++++++----- src/tui.ts | 12 ++++++++- tests/context-injection.test.js | 6 ++--- tests/honcho-setup.test.js | 48 +++++++++++++++++++++++++++++++++ tests/tui-behavior.test.js | 1 + tests/user-peer-id.test.js | 13 ++++----- 6 files changed, 99 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index d2f2684..1c66500 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,9 +26,12 @@ type HonchoSettings = { workspace: string recallMode: RecallMode sessionStrategy: SessionStrategy + removeUserPrefix: boolean } -type HostScopedSettings = Partial> +type HostScopedSettings = Partial< + Pick +> type RuntimeHandle = { rootDir: string @@ -108,6 +111,10 @@ const DEFAULT_SETTINGS: HonchoSettings = { workspace: "opencode", recallMode: "hybrid", sessionStrategy: "per-directory", + // Default false for everyone, including upgrading installs: an existing user + // keeps their `user-` peer and its memory. New installs are stamped + // true at config creation, and anyone can opt in by setting it true. + removeUserPrefix: false, } const INTERNAL_DIALECTIC_REASONING_LEVEL: DialecticReasoningLevel = "low" @@ -121,7 +128,7 @@ const INTERNAL_CONTEXT_REFRESH: ContextRefreshSettings = { useSessionStartDialectic: true, } -const BOOLEAN_KEYS = new Set([]) +const BOOLEAN_KEYS = new Set(["removeUserPrefix"]) const NUMBER_KEYS = new Set([]) @@ -133,7 +140,13 @@ const ENUM_KEYS: Record> = { const INHERITABLE_STRING_KEYS = new Set(["apiKey", "baseUrl", "peerName", "aiPeer", "workspace"]) const TOP_LEVEL_SETTING_FIELDS = new Set(["apiKey", "baseUrl", "peerName"]) -const HOST_SETTING_FIELDS = new Set(["workspace", "aiPeer", "recallMode", "sessionStrategy"]) +const HOST_SETTING_FIELDS = new Set([ + "workspace", + "aiPeer", + "recallMode", + "sessionStrategy", + "removeUserPrefix", +]) const SETTING_FIELD_PATHS = new Set([ "apiKey", @@ -143,6 +156,7 @@ const SETTING_FIELD_PATHS = new Set([ "workspace", "recallMode", "sessionStrategy", + "removeUserPrefix", ]) const DURABLE_PATTERNS = [ @@ -616,8 +630,13 @@ const writeSettings = async ( const currentUserName = () => "user" -const deriveUserPeerId = (settings: Pick) => - normalizeId(settings.peerName || currentUserName()) +const deriveUserPeerId = (settings: Pick) => { + const name = settings.peerName || currentUserName() + // removeUserPrefix=true drops the `user-` prefix to match the sibling + // claude-honcho / hermes-honcho plugins; false (the legacy-safe default) + // keeps the historical `user-` peer and its accumulated memory. + return settings.removeUserPrefix ? normalizeId(name) : normalizeId(`user:${name}`) +} const assertDistinctUserAndAgentPeers = (userPeerId: string, rootAgentPeerId: string) => { if (userPeerId === rootAgentPeerId) { @@ -640,6 +659,7 @@ const hostDefaults = (settings: HonchoSettings): Record => { aiPeer, recallMode: settings.recallMode, sessionStrategy: settings.sessionStrategy, + removeUserPrefix: settings.removeUserPrefix, } } @@ -664,11 +684,15 @@ const ensureSharedGlobalSettings = async (configPath = sharedGlobalSettingsPath( if (sharedRaw) { next = currentShared } else { + // Brand-new install (no prior config): ship removeUserPrefix=true so new + // users get the bare `` peer. Existing configs take the `if` branch + // untouched and fall back to the false default, preserving their `user-` + // peer. next = { peerName: currentUserName(), baseUrl: mergedHostSettings.baseUrl, hosts: { - opencode: hostDefaults(mergedHostSettings), + opencode: { ...hostDefaults(mergedHostSettings), removeUserPrefix: true }, }, } await writeSharedGlobalSettings(configPath, next) @@ -1074,6 +1098,7 @@ export const createHonchoRuntimePlugin = recallMode: handle.config.recallMode, sessionStrategy: handle.config.sessionStrategy, peerName: handle.config.peerName, + removeUserPrefix: handle.config.removeUserPrefix, configured: hasConfiguredAuth(handle.config), localMode: isLocalBaseUrl(handle.config.baseUrl), baseUrl: handle.config.baseUrl, diff --git a/src/tui.ts b/src/tui.ts index 47764e6..3bcec54 100644 --- a/src/tui.ts +++ b/src/tui.ts @@ -24,6 +24,7 @@ const MODE_EDITABLE_FIELD_PATHS = [ "hosts.opencode.aiPeer", "hosts.opencode.recallMode", "hosts.opencode.sessionStrategy", + "hosts.opencode.removeUserPrefix", ] as const type GlobalSettings = { @@ -36,6 +37,7 @@ type GlobalSettings = { aiPeer?: string recallMode?: "hybrid" | "context" | "tools" sessionStrategy?: "per-repo" | "per-directory" | "per-session" | "global" | "git-branch" | "chat-instance" + removeUserPrefix?: boolean } } } @@ -240,7 +242,8 @@ const settingsMessage = (settings: GlobalSettings) => { const saveSettings = async (partial: Partial) => { const current = await readGlobalSettings() - const sharedRaw = (await readSharedConfig()) ?? {} + const existingSharedConfig = await readSharedConfig() + const sharedRaw = existingSharedConfig ?? {} const partialHost = partial.hosts?.opencode const nextApiKey = typeof partial.apiKey === "string" @@ -256,12 +259,19 @@ const saveSettings = async (partial: Partial) => { : "user" const currentHosts = isRecord(sharedRaw.hosts) ? { ...sharedRaw.hosts } : {} const currentOpenCodeHost = isRecord(currentHosts.opencode) ? currentHosts.opencode : {} + // New install (no prior shared config) ships removeUserPrefix=true; an existing + // config keeps whatever it had, defaulting to false so upgraders aren't moved + // off their `user-` peer. + const existingRemoveUserPrefix = current.hosts?.opencode?.removeUserPrefix currentHosts.opencode = { ...currentOpenCodeHost, workspace: partialHost?.workspace ?? current.hosts?.opencode?.workspace ?? "opencode", aiPeer: partialHost?.aiPeer ?? current.hosts?.opencode?.aiPeer ?? "opencode", recallMode: partialHost?.recallMode ?? current.hosts?.opencode?.recallMode ?? "hybrid", sessionStrategy: partialHost?.sessionStrategy ?? current.hosts?.opencode?.sessionStrategy ?? "per-directory", + removeUserPrefix: + partialHost?.removeUserPrefix ?? + (typeof existingRemoveUserPrefix === "boolean" ? existingRemoveUserPrefix : !existingSharedConfig), } const next: GlobalSettings & Record = { diff --git a/tests/context-injection.test.js b/tests/context-injection.test.js index 0c5e9d6..8bfe17e 100644 --- a/tests/context-injection.test.js +++ b/tests/context-injection.test.js @@ -96,9 +96,9 @@ const createHonchoFetch = ({ failStableHydration = false } = {}) => { return jsonResponse({ peer_id: peerId, target_id: null, - representation: peerId === "user" - ? "The user prefers concise engineering analysis." - : "The assistant is working on opencode-honcho.", + representation: peerId === "opencode" + ? "The assistant is working on opencode-honcho." + : "The user prefers concise engineering analysis.", peer_card: ["Keep changes narrowly scoped."], }) } diff --git a/tests/honcho-setup.test.js b/tests/honcho-setup.test.js index ca30435..a198fa9 100644 --- a/tests/honcho-setup.test.js +++ b/tests/honcho-setup.test.js @@ -131,6 +131,7 @@ test("honcho_setup writes shared Honcho config with root peerName and hosts.open workspace: "opencode", recallMode: "hybrid", sessionStrategy: "per-directory", + removeUserPrefix: true, }) expect("linkedHosts" in persisted.hosts.opencode).toBe(false) expect(result.persistedFields).toContain("peerName") @@ -668,3 +669,50 @@ test("honcho_status uses the project worktree to derive per-directory session ke expect(result.sessionKey).toBe("per-directory-opencode-services-api-opencode") }) }) + +test("fresh install ships removeUserPrefix=true and drops the user- prefix", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "honcho-fresh-prefix-root-")) + const homeDir = await mkdtemp(path.join(os.tmpdir(), "honcho-fresh-prefix-home-")) + const sharedConfigPath = path.join(homeDir, ".honcho", "config.json") + + await withEnv( + { HOME: homeDir, USER: "ignored-user", XDG_CONFIG_HOME: undefined, HONCHO_PEER_NAME: "alice" }, + async () => { + const hooks = await createPluginHarness(rootDir) + const result = JSON.parse(await hooks.tool.honcho_status.execute({}, toolContext(rootDir))) + const persisted = JSON.parse(await readFile(sharedConfigPath, "utf-8")) + + expect(result.removeUserPrefix).toBe(true) + expect(result.peers.userPeer.id).toBe("alice") + expect(persisted.hosts.opencode.removeUserPrefix).toBe(true) + }, + ) +}) + +test("upgrading install (existing config) keeps removeUserPrefix=false and the user- prefix", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "honcho-legacy-prefix-root-")) + const homeDir = await mkdtemp(path.join(os.tmpdir(), "honcho-legacy-prefix-home-")) + const sharedConfigDir = path.join(homeDir, ".honcho") + const sharedConfigPath = path.join(sharedConfigDir, "config.json") + + // Config predates removeUserPrefix — the false default must apply and the file + // must not be mutated on a plain status read. + await mkdir(sharedConfigDir, { recursive: true }) + const initialConfig = { + peerName: "alice", + apiKey: "legacy-key", + baseUrl: "https://api.honcho.dev", + hosts: { opencode: { aiPeer: "opencode", workspace: "opencode" } }, + } + await writeFile(sharedConfigPath, JSON.stringify(initialConfig, null, 2)) + + await withEnv({ HOME: homeDir, USER: "ignored-user", XDG_CONFIG_HOME: undefined }, async () => { + const hooks = await createPluginHarness(rootDir) + const result = JSON.parse(await hooks.tool.honcho_status.execute({}, toolContext(rootDir))) + const persisted = JSON.parse(await readFile(sharedConfigPath, "utf-8")) + + expect(result.removeUserPrefix).toBe(false) + expect(result.peers.userPeer.id).toBe("user-alice") + expect(persisted).toEqual(initialConfig) + }) +}) diff --git a/tests/tui-behavior.test.js b/tests/tui-behavior.test.js index c85da4f..3c5e868 100644 --- a/tests/tui-behavior.test.js +++ b/tests/tui-behavior.test.js @@ -113,6 +113,7 @@ test("honcho config only exposes top-level and hosts.opencode fields", () => { "hosts.opencode.aiPeer", "hosts.opencode.recallMode", "hosts.opencode.sessionStrategy", + "hosts.opencode.removeUserPrefix", ]) assert.equal(__testing.modeEditableFieldPaths().includes("hosts.claude_code.workspace"), false) assert.equal(__testing.modeEditableFieldPaths().includes("hosts.other.aiPeer"), false) diff --git a/tests/user-peer-id.test.js b/tests/user-peer-id.test.js index fb29f6c..f9cbffc 100644 --- a/tests/user-peer-id.test.js +++ b/tests/user-peer-id.test.js @@ -2,14 +2,11 @@ import { expect, test } from "bun:test" import { __testing } from "../dist/index.js" -test("user peer id is the bare peerName with no prefix", () => { - expect(__testing.deriveUserPeerId({ peerName: "rui" })).toBe("rui") +test("removeUserPrefix=false (the default) keeps the legacy user- prefix", () => { + expect(__testing.deriveUserPeerId({ peerName: "rui", removeUserPrefix: false })).toBe("user-rui") + expect(__testing.deriveUserPeerId({ peerName: "rui" })).toBe("user-rui") }) -test("user peer id falls back to the current user name when peerName is empty", () => { - expect(__testing.deriveUserPeerId({ peerName: "" })).toBe("user") -}) - -test("user peer id is normalised", () => { - expect(__testing.deriveUserPeerId({ peerName: "Rui Rei" })).toBe("rui-rei") +test("removeUserPrefix=true drops the prefix", () => { + expect(__testing.deriveUserPeerId({ peerName: "rui", removeUserPrefix: true })).toBe("rui") }) From 09dc78af9bea5b15c3f2a3e7550830563907e282 Mon Sep 17 00:00:00 2001 From: adavyas Date: Mon, 15 Jun 2026 19:59:00 -0400 Subject: [PATCH 4/7] docs: mention removeUserPrefix in the config example Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 47ab5b1..836b2ff 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,8 @@ OpenCode reads and writes this shared config file directly. OpenCode-specific de "workspace": "opencode", "aiPeer": "opencode", "recallMode": "hybrid", - "sessionStrategy": "per-directory" + "sessionStrategy": "per-directory", + "removeUserPrefix": true // true uses the bare peerName; false (default on upgrade) keeps the legacy user- peer } } } From da8ac9cbfaf0d658474acad267118484f878aa08 Mon Sep 17 00:00:00 2001 From: adavyas Date: Mon, 15 Jun 2026 20:23:58 -0400 Subject: [PATCH 5/7] fix: coerce removeUserPrefix to boolean when editing an absent field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TUI value parser keyed "is this boolean?" off the runtime type of the current value. For upgrade configs where hosts.opencode.removeUserPrefix is absent, currentValue is undefined, so editing it persisted the raw string "true"/"false" instead of a boolean — and since "false" is a truthy non-empty string, deriveUserPeerId would route to the bare peer, the opposite of intended. Decide boolean-ness from the field path (BOOLEAN_FIELD_KEYS) so both the true/false options and the coercion are correct even when the field is missing. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tui.ts | 23 ++++++++++++++++++----- tests/tui-behavior.test.js | 8 ++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/tui.ts b/src/tui.ts index 3bcec54..4d88c8c 100644 --- a/src/tui.ts +++ b/src/tui.ts @@ -139,20 +139,32 @@ const resolveSharedConfigField = (config: Record, field: string const modeEditableFieldPaths = () => [...MODE_EDITABLE_FIELD_PATHS] +// Fields that are always booleans, keyed by their final path segment. Editing +// these must coerce to a real boolean even when the field is currently absent +// (e.g. upgrade configs without removeUserPrefix), where currentValue is +// undefined and a runtime-type check alone would persist the raw "true"/"false" +// string. +const BOOLEAN_FIELD_KEYS = new Set(["removeuserprefix"]) + +const fieldKey = (fieldPath: string) => fieldPath.split(".").at(-1)?.toLowerCase() || fieldPath.toLowerCase() + +const isBooleanField = (fieldPath: string, currentValue: unknown) => + typeof currentValue === "boolean" || BOOLEAN_FIELD_KEYS.has(fieldKey(fieldPath)) + const sharedConfigPresetOptions = (fieldPath: string, currentValue: unknown) => { - const presetKey = fieldPath.split(".").at(-1)?.toLowerCase() || fieldPath.toLowerCase() + const presetKey = fieldKey(fieldPath) if (SHARED_CONFIG_PRESETS[presetKey]) { return [...SHARED_CONFIG_PRESETS[presetKey]] } - if (typeof currentValue === "boolean") { + if (isBooleanField(fieldPath, currentValue)) { return ["true", "false"] } return [] } -const parseSharedConfigValue = (currentValue: unknown, rawValue: string) => { +const parseSharedConfigValue = (fieldPath: string, currentValue: unknown, rawValue: string) => { const trimmed = rawValue.trim() - if (typeof currentValue === "boolean") { + if (isBooleanField(fieldPath, currentValue)) { return trimmed.toLowerCase() === "true" } if (typeof currentValue === "number") { @@ -416,7 +428,7 @@ const openModeValueDialog = async ( const persistValue = async (rawValue: string) => { try { const nextConfig = structuredClone(config) - const nextValue = parseSharedConfigValue(currentValue, rawValue) + const nextValue = parseSharedConfigValue(fieldPath, currentValue, rawValue) setNestedValue(nextConfig, fieldPath, nextValue) const configPath = await writeSharedConfig(nextConfig) api.ui.dialog.replace(() => @@ -605,6 +617,7 @@ export const __testing = { modeEditableFieldPaths, readSharedConfig, resolveSharedConfigField, + parseSharedConfigValue, saveSettings, settingsMessage, sharedConfigPath, diff --git a/tests/tui-behavior.test.js b/tests/tui-behavior.test.js index 3c5e868..05b2bd2 100644 --- a/tests/tui-behavior.test.js +++ b/tests/tui-behavior.test.js @@ -119,6 +119,14 @@ test("honcho config only exposes top-level and hosts.opencode fields", () => { assert.equal(__testing.modeEditableFieldPaths().includes("hosts.other.aiPeer"), false) }) +test("editing removeUserPrefix persists a boolean even when the field is initially absent", () => { + // Upgrade config: field missing -> currentValue is undefined. + assert.strictEqual(__testing.parseSharedConfigValue("hosts.opencode.removeUserPrefix", undefined, "true"), true) + assert.strictEqual(__testing.parseSharedConfigValue("hosts.opencode.removeUserPrefix", undefined, "false"), false) + // And it still offers true/false options when the value is absent. + assert.deepEqual(__testing.sharedConfigPresetOptions("hosts.opencode.removeUserPrefix", undefined), ["true", "false"]) +}) + test("shared config field resolution is case-insensitive and preserves canonical paths", () => { const field = __testing.resolveSharedConfigField( { From 7c92961d57f48227e64e4ccd0310f66a140afed3 Mon Sep 17 00:00:00 2001 From: adavyas Date: Tue, 16 Jun 2026 15:38:05 -0400 Subject: [PATCH 6/7] fix: coerce removeUserPrefix at read time and soften peer collision Two real hardening fixes from code review (cosmetic findings dropped): - applyRawLayer coerces BOOLEAN_KEYS at the merge chokepoint, so a config string "false" is no longer truthy. The README invites hand-editing, and a stray string "false" would otherwise drop the user- prefix and orphan a user's memory. - deriveRuntimeHandle auto-resolves a bare/agent peer-id collision (peerName == aiPeer) by falling back to the prefixed form instead of throwing on the hot path, which runs in unguarded hooks. assertDistinct stays as a last resort. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 17 ++++++++++++++++- tests/honcho-setup.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 1c66500..27b0463 100644 --- a/src/index.ts +++ b/src/index.ts @@ -369,6 +369,14 @@ const applyRawLayer = (target: HonchoSettings, raw: Record) => if (value === undefined || value === null) { continue } + if (BOOLEAN_KEYS.has(key)) { + // Coerce booleans at the read boundary: a config (or hand-edit, which the + // README invites) may carry the string "true"/"false". Only true/"true" is + // true, so a stray "false" can't be truthy and silently flip the prefix. + ;(target as Record)[key] = + value === true || (typeof value === "string" && value.trim().toLowerCase() === "true") + continue + } if (typeof value === "string") { const expanded = expandEnv(value) if (INHERITABLE_STRING_KEYS.has(key) && !expanded.trim()) { @@ -789,8 +797,15 @@ const deriveRuntimeHandle = async ( const sessionId = extractSessionId(input) const repoName = path.basename(rootDir) const workspaceId = normalizeId(settings.workspace || "opencode") - const userPeerId = deriveUserPeerId(settings) const rootAgentPeerId = normalizeId(settings.aiPeer || "opencode") + // If the bare form collides with the agent peer (peerName === aiPeer), fall back + // to the prefixed form to keep user and agent memory distinct, rather than + // throwing on this hot path (deriveRuntimeHandle runs in unguarded hooks). + // assertDistinct only fires for a genuinely unresolvable config. + let userPeerId = deriveUserPeerId(settings) + if (userPeerId === rootAgentPeerId) { + userPeerId = normalizeId(`user:${settings.peerName || currentUserName()}`) + } assertDistinctUserAndAgentPeers(userPeerId, rootAgentPeerId) const activeAgentPeerId = rootAgentPeerId const childAgentPeerId = null diff --git a/tests/honcho-setup.test.js b/tests/honcho-setup.test.js index a198fa9..8507b76 100644 --- a/tests/honcho-setup.test.js +++ b/tests/honcho-setup.test.js @@ -716,3 +716,30 @@ test("upgrading install (existing config) keeps removeUserPrefix=false and the u expect(persisted).toEqual(initialConfig) }) }) + +test("a string removeUserPrefix is coerced at read time (a stray \"false\" stays on the prefixed peer)", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "honcho-coerce-root-")) + const homeDir = await mkdtemp(path.join(os.tmpdir(), "honcho-coerce-home-")) + await mkdir(path.join(homeDir, ".honcho"), { recursive: true }) + const cfg = path.join(homeDir, ".honcho", "config.json") + + await withEnv({ HOME: homeDir, USER: "ignored-user", XDG_CONFIG_HOME: undefined }, async () => { + await writeFile(cfg, JSON.stringify({ peerName: "alice", hosts: { opencode: { removeUserPrefix: "false" } } })) + const result = JSON.parse(await (await createPluginHarness(rootDir)).tool.honcho_status.execute({}, toolContext(rootDir))) + expect(result.peers.userPeer.id).toBe("user-alice") + }) +}) + +test("bare peer colliding with the agent peer falls back to the prefix instead of throwing", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "honcho-collide-root-")) + const homeDir = await mkdtemp(path.join(os.tmpdir(), "honcho-collide-home-")) + await mkdir(path.join(homeDir, ".honcho"), { recursive: true }) + const cfg = path.join(homeDir, ".honcho", "config.json") + + await withEnv({ HOME: homeDir, USER: "ignored-user", XDG_CONFIG_HOME: undefined }, async () => { + await writeFile(cfg, JSON.stringify({ peerName: "opencode", hosts: { opencode: { aiPeer: "opencode", removeUserPrefix: true } } })) + const result = JSON.parse(await (await createPluginHarness(rootDir)).tool.honcho_status.execute({}, toolContext(rootDir))) + expect(result.ok).toBe(true) + expect(result.peers.userPeer.id).toBe("user-opencode") + }) +}) From cdb538ab5190b6626d2394c5c5e693277a1222ef Mon Sep 17 00:00:00 2001 From: adavyas Date: Wed, 17 Jun 2026 12:30:24 -0400 Subject: [PATCH 7/7] fix: drop TUI surface for removeUserPrefix, keep it config-only removeUserPrefix is now managed purely through the shared config file (and the runtime new-install stamp / honcho_set_config), not the native TUI editor. Revert src/tui.ts and tests/tui-behavior.test.js to their pre-feature state. The TUI's saveSettings still preserves the key on existing configs via the currentOpenCodeHost spread, so configs that set it aren't disturbed when other settings are edited. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tui.ts | 35 ++++++----------------------------- tests/tui-behavior.test.js | 9 --------- 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/src/tui.ts b/src/tui.ts index 4d88c8c..47764e6 100644 --- a/src/tui.ts +++ b/src/tui.ts @@ -24,7 +24,6 @@ const MODE_EDITABLE_FIELD_PATHS = [ "hosts.opencode.aiPeer", "hosts.opencode.recallMode", "hosts.opencode.sessionStrategy", - "hosts.opencode.removeUserPrefix", ] as const type GlobalSettings = { @@ -37,7 +36,6 @@ type GlobalSettings = { aiPeer?: string recallMode?: "hybrid" | "context" | "tools" sessionStrategy?: "per-repo" | "per-directory" | "per-session" | "global" | "git-branch" | "chat-instance" - removeUserPrefix?: boolean } } } @@ -139,32 +137,20 @@ const resolveSharedConfigField = (config: Record, field: string const modeEditableFieldPaths = () => [...MODE_EDITABLE_FIELD_PATHS] -// Fields that are always booleans, keyed by their final path segment. Editing -// these must coerce to a real boolean even when the field is currently absent -// (e.g. upgrade configs without removeUserPrefix), where currentValue is -// undefined and a runtime-type check alone would persist the raw "true"/"false" -// string. -const BOOLEAN_FIELD_KEYS = new Set(["removeuserprefix"]) - -const fieldKey = (fieldPath: string) => fieldPath.split(".").at(-1)?.toLowerCase() || fieldPath.toLowerCase() - -const isBooleanField = (fieldPath: string, currentValue: unknown) => - typeof currentValue === "boolean" || BOOLEAN_FIELD_KEYS.has(fieldKey(fieldPath)) - const sharedConfigPresetOptions = (fieldPath: string, currentValue: unknown) => { - const presetKey = fieldKey(fieldPath) + const presetKey = fieldPath.split(".").at(-1)?.toLowerCase() || fieldPath.toLowerCase() if (SHARED_CONFIG_PRESETS[presetKey]) { return [...SHARED_CONFIG_PRESETS[presetKey]] } - if (isBooleanField(fieldPath, currentValue)) { + if (typeof currentValue === "boolean") { return ["true", "false"] } return [] } -const parseSharedConfigValue = (fieldPath: string, currentValue: unknown, rawValue: string) => { +const parseSharedConfigValue = (currentValue: unknown, rawValue: string) => { const trimmed = rawValue.trim() - if (isBooleanField(fieldPath, currentValue)) { + if (typeof currentValue === "boolean") { return trimmed.toLowerCase() === "true" } if (typeof currentValue === "number") { @@ -254,8 +240,7 @@ const settingsMessage = (settings: GlobalSettings) => { const saveSettings = async (partial: Partial) => { const current = await readGlobalSettings() - const existingSharedConfig = await readSharedConfig() - const sharedRaw = existingSharedConfig ?? {} + const sharedRaw = (await readSharedConfig()) ?? {} const partialHost = partial.hosts?.opencode const nextApiKey = typeof partial.apiKey === "string" @@ -271,19 +256,12 @@ const saveSettings = async (partial: Partial) => { : "user" const currentHosts = isRecord(sharedRaw.hosts) ? { ...sharedRaw.hosts } : {} const currentOpenCodeHost = isRecord(currentHosts.opencode) ? currentHosts.opencode : {} - // New install (no prior shared config) ships removeUserPrefix=true; an existing - // config keeps whatever it had, defaulting to false so upgraders aren't moved - // off their `user-` peer. - const existingRemoveUserPrefix = current.hosts?.opencode?.removeUserPrefix currentHosts.opencode = { ...currentOpenCodeHost, workspace: partialHost?.workspace ?? current.hosts?.opencode?.workspace ?? "opencode", aiPeer: partialHost?.aiPeer ?? current.hosts?.opencode?.aiPeer ?? "opencode", recallMode: partialHost?.recallMode ?? current.hosts?.opencode?.recallMode ?? "hybrid", sessionStrategy: partialHost?.sessionStrategy ?? current.hosts?.opencode?.sessionStrategy ?? "per-directory", - removeUserPrefix: - partialHost?.removeUserPrefix ?? - (typeof existingRemoveUserPrefix === "boolean" ? existingRemoveUserPrefix : !existingSharedConfig), } const next: GlobalSettings & Record = { @@ -428,7 +406,7 @@ const openModeValueDialog = async ( const persistValue = async (rawValue: string) => { try { const nextConfig = structuredClone(config) - const nextValue = parseSharedConfigValue(fieldPath, currentValue, rawValue) + const nextValue = parseSharedConfigValue(currentValue, rawValue) setNestedValue(nextConfig, fieldPath, nextValue) const configPath = await writeSharedConfig(nextConfig) api.ui.dialog.replace(() => @@ -617,7 +595,6 @@ export const __testing = { modeEditableFieldPaths, readSharedConfig, resolveSharedConfigField, - parseSharedConfigValue, saveSettings, settingsMessage, sharedConfigPath, diff --git a/tests/tui-behavior.test.js b/tests/tui-behavior.test.js index 05b2bd2..c85da4f 100644 --- a/tests/tui-behavior.test.js +++ b/tests/tui-behavior.test.js @@ -113,20 +113,11 @@ test("honcho config only exposes top-level and hosts.opencode fields", () => { "hosts.opencode.aiPeer", "hosts.opencode.recallMode", "hosts.opencode.sessionStrategy", - "hosts.opencode.removeUserPrefix", ]) assert.equal(__testing.modeEditableFieldPaths().includes("hosts.claude_code.workspace"), false) assert.equal(__testing.modeEditableFieldPaths().includes("hosts.other.aiPeer"), false) }) -test("editing removeUserPrefix persists a boolean even when the field is initially absent", () => { - // Upgrade config: field missing -> currentValue is undefined. - assert.strictEqual(__testing.parseSharedConfigValue("hosts.opencode.removeUserPrefix", undefined, "true"), true) - assert.strictEqual(__testing.parseSharedConfigValue("hosts.opencode.removeUserPrefix", undefined, "false"), false) - // And it still offers true/false options when the value is absent. - assert.deepEqual(__testing.sharedConfigPresetOptions("hosts.opencode.removeUserPrefix", undefined), ["true", "false"]) -}) - test("shared config field resolution is case-insensitive and preserves canonical paths", () => { const field = __testing.resolveSharedConfigField( {