diff --git a/docs/task-agent-discovery.md b/docs/task-agent-discovery.md index 05cff512d..3cf5bf9c6 100644 --- a/docs/task-agent-discovery.md +++ b/docs/task-agent-discovery.md @@ -71,7 +71,7 @@ modelRoles: review: openai/gpt-5.4:high ``` -`@review` resolves through `modelRoles.review`. Each `modelRoles.` value stores a concrete model selector and may append a thinking suffix such as `:high` (`src/config/model-resolver.ts`). Changing that mapping affects subsequent task resolutions without editing agent definitions. +`@review` resolves through `modelRoles.review`. Each `modelRoles.` value stores a concrete model selector and may append a thinking suffix such as `:high` (`src/config/model-resolver.ts`). Changing that mapping affects subsequent task resolutions without editing agent definitions. Task/eval preflight reloads the current global, project, and explicit overlay settings before rediscovering agents, so agent files and their role aliases added during a live session resolve from one refreshed configuration state. For a dispatch, set the agent name and task: @@ -185,11 +185,12 @@ Lookup is exact-name linear search: `resolveEffectiveSubagentPolicy()` is shared by task and eval-backed subagent launches. Before allocating artifacts it: -1. resolves the omitted or explicit agent name from the parent spawn policy -2. enforces depth, blocked-self-recursion, and parent spawn-policy guards -3. rediscovers agents with `discoverAgents(session.cwd)` and performs exact lookup -4. checks `task.disabledAgents` -5. resolves plan-mode restrictions, output schema, model policy, and isolation policy +1. atomically reloads the live session's persisted global, project, and explicit overlay settings while preserving runtime overrides +2. resolves the omitted or explicit agent name from the parent spawn policy +3. enforces depth, blocked-self-recursion, and parent spawn-policy guards +4. rediscovers agents with `discoverAgents(session.cwd)` and performs exact lookup +5. checks `task.disabledAgents` +6. resolves plan-mode restrictions, output schema, model policy, and isolation policy A missing name fails preflight with `Unknown agent "...". Available: ...`; no subprocess runs. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4714e83fc..a96441ccc 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,6 +8,9 @@ ### Fixed - Fixed Claude Code marketplace plugins ignoring the `enabledPlugins` switch in `~/.claude/settings.json` and `.claude/settings(.local).json`: a plugin turned off for a project no longer loads there, and a local-scope install enabled for a project loads even when its recorded `projectPath` is a different directory +### Fixed + +- Fixed task and eval subagents discovering newly added agent definitions while resolving their role aliases from stale startup settings. Subagent preflight now atomically reloads persisted settings before agent discovery while preserving live runtime overrides. ## [17.3.7] - 2026-08-17 diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index f4d55e10c..598c65bf1 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -71,6 +71,22 @@ type YamlLoadResult = | { kind: "invalid"; error: unknown; backupPath?: string } | { kind: "unreadable"; error: unknown }; +type MainYamlReadResult = { + settings: RawSettings | null; + configPath: string | null; +}; + +type ProjectSettingsReadResult = { + settings: RawSettings; + fileSettings: RawSettings; + shellPathSource: string | undefined; +}; + +type ConfigOverlayReadResult = { + settings: RawSettings; + shellPathSource: string | undefined; +}; + export interface SettingsOptions { /** Current working directory for project settings discovery */ cwd?: string; @@ -353,6 +369,8 @@ export class Settings { #modifiedProjectModelRoles = new Set(); /** Individual global model roles modified during this session (for partial save) */ #modifiedGlobalModelRoles = new Set(); + /** Changes whenever a live API mutates a persisted layer. */ + #persistedMutationGeneration = 0; /** * Original process-wide model-role overrides captured before a project edit * temporarily replaced them via `#updateRuntimeModelRoleOverride`. Restored @@ -370,6 +388,8 @@ export class Settings { #savePromise?: Promise; #projectSaveTimer?: NodeJS.Timeout; #projectSavePromise?: Promise; + /** Coalesces concurrent persisted-layer refreshes into one atomic reload. */ + #reloadFromDiskPromise?: Promise; /** Whether to persist changes */ #persist: boolean; @@ -503,6 +523,7 @@ export class Settings { const prev = this.get(path); const segments = path.split("."); setByPath(this.#global, segments, value); + this.#persistedMutationGeneration++; this.#modified.add(path); this.#rebuildMerged(); const next = this.get(path); @@ -624,6 +645,83 @@ export class Settings { return cloned; } + /** + * Re-read the current global, project, and explicit overlay layers from disk + * without replacing this instance or discarding runtime overrides. + * + * All sources are loaded before any live layer is replaced, so readers never + * observe a partially refreshed configuration. Concurrent callers share the + * same reload. + */ + async reloadFromDisk(): Promise { + if (!this.#persist) return; + if (this.#reloadFromDiskPromise) return this.#reloadFromDiskPromise; + + const reload = this.#reloadPersistedLayers(); + this.#reloadFromDiskPromise = reload; + try { + await reload; + } finally { + if (this.#reloadFromDiskPromise === reload) { + this.#reloadFromDiskPromise = undefined; + } + } + } + + async #reloadPersistedLayers(): Promise { + for (;;) { + await this.flush(); + const mutationGeneration = this.#persistedMutationGeneration; + const previousSignaledValues = { + modelRoles: this.get("modelRoles"), + sessionAccent: this.get("statusLine.sessionAccent"), + }; + const previousHookValues = new Map(); + for (const key of Object.keys(SETTING_HOOKS) as SettingPath[]) { + previousHookValues.set(key, this.get(key)); + } + + const [globalResult, projectResult, overlayResult] = await Promise.allSettled([ + this.#readExistingMainYaml(false), + this.#readProjectSettings(false), + this.#readConfigOverlays(false), + ]); + if (mutationGeneration !== this.#persistedMutationGeneration) continue; + if (globalResult.status === "rejected") throw globalResult.reason; + if (projectResult.status === "rejected") throw projectResult.reason; + if (overlayResult.status === "rejected") throw overlayResult.reason; + + this.#configPath = globalResult.value.configPath; + this.#global = globalResult.value.settings ?? {}; + this.#project = projectResult.value.settings; + this.#projectFileSettings = projectResult.value.fileSettings; + this.#projectShellPathSource = projectResult.value.shellPathSource; + this.#configOverlay = overlayResult.value.settings; + this.#overlayShellPathSource = overlayResult.value.shellPathSource; + this.#rebuildMerged(); + + const nextModelRoles = this.get("modelRoles"); + if (!Bun.deepEquals(nextModelRoles, previousSignaledValues.modelRoles)) { + this.#fireEffectiveSettingChanged("modelRoles", nextModelRoles, previousSignaledValues.modelRoles); + } + const nextSessionAccent = this.get("statusLine.sessionAccent"); + if (!Bun.deepEquals(nextSessionAccent, previousSignaledValues.sessionAccent)) { + this.#fireEffectiveSettingChanged( + "statusLine.sessionAccent", + nextSessionAccent, + previousSignaledValues.sessionAccent, + ); + } + for (const [key, previous] of previousHookValues) { + const next = this.get(key); + if (!Bun.deepEquals(next, previous)) { + SETTING_HOOKS[key]?.(next, previous); + } + } + return; + } + } + /** * Re-scope this instance to a new working directory *in place*: reload the * project layer (`.claude/settings.yml` etc.) from `cwd`, re-resolve @@ -870,6 +968,7 @@ export class Settings { current[role] = modelId; setByPath(this.#project, ["modelRoles"], current); this.#modifiedProjectModelRoles.add(role); + this.#persistedMutationGeneration++; this.#rebuildMerged(); this.#fireEffectiveSettingChanged("modelRoles", this.get("modelRoles"), prev); this.#queueProjectSave(); @@ -903,6 +1002,7 @@ export class Settings { // clobbered by this process's stale in-memory snapshot. setByPath(this.#global, ["modelRoles"], current); this.#modifiedGlobalModelRoles.add(role); + this.#persistedMutationGeneration++; this.#rebuildMerged(); this.#queueSave(); this.#fireEffectiveSettingChanged("modelRoles", this.get("modelRoles"), prev); @@ -1091,7 +1191,7 @@ export class Settings { return loaded ?? {}; } - async #loadYamlIfPresent(filePath: string): Promise { + async #loadYamlIfPresent(filePath: string, captureLegacyChangelogVersion = true): Promise { let content: string; try { content = await fs.promises.readFile(filePath, "utf8"); @@ -1115,7 +1215,10 @@ export class Settings { error: new Error("Settings YAML must contain a mapping at the document root"), }; } - return { kind: "loaded", settings: this.#migrateRawSettings(parsed as RawSettings) }; + return { + kind: "loaded", + settings: this.#migrateRawSettings(parsed as RawSettings, captureLegacyChangelogVersion), + }; } async #resolveYamlWritePath(filePath: string): Promise { @@ -1215,55 +1318,81 @@ export class Settings { } } - async #loadExistingMainYaml(): Promise { - if (!this.#configPath) return null; + async #readExistingMainYaml(quarantineInvalid: boolean): Promise { + if (!this.#configPath) return { settings: null, configPath: null }; for (const filename of MAIN_CONFIG_FILENAMES) { const configPath = path.join(this.#agentDir, filename); - const loaded = await this.#loadYamlIfPresentForStartup(configPath); - if (loaded) { - this.#configPath = configPath; - return loaded; - } + const loaded = quarantineInvalid + ? await this.#loadYamlIfPresentForStartup(configPath) + : this.#unwrapYamlLoadResult(configPath, await this.#loadYamlIfPresent(configPath, false)); + if (loaded) return { settings: loaded, configPath }; } - this.#configPath = path.join(this.#agentDir, MAIN_CONFIG_FILENAMES[0]); - return null; + return { + settings: null, + configPath: path.join(this.#agentDir, MAIN_CONFIG_FILENAMES[0]), + }; } - async #loadProjectSettings(): Promise { - this.#projectShellPathSource = undefined; + async #loadExistingMainYaml(): Promise { + const result = await this.#readExistingMainYaml(true); + this.#configPath = result.configPath; + return result.settings; + } + + async #readProjectSettings(quarantineInvalid: boolean): Promise { + let shellPathSource: string | undefined; let merged: RawSettings = {}; try { const result = await loadCapability(settingsCapability.id, { cwd: this.#cwd }); for (const item of result.items as SettingsCapabilityItem[]) { if (item.level === "project") { merged = this.#deepMerge(merged, item.data as RawSettings); - if (Object.hasOwn(item.data, "shellPath")) this.#projectShellPathSource = item.path; + if (Object.hasOwn(item.data, "shellPath")) shellPathSource = item.path; } } } catch { - this.#projectShellPathSource = undefined; + shellPathSource = undefined; // Capability discovery is best-effort; the native project config below // remains authoritative for its model-role layer and must not be hidden. } const projectConfigPath = path.join(this.#cwd, ".omp", "config.yml"); - const nativeProject = await this.#loadYaml(projectConfigPath); - this.#projectFileSettings = structuredClone(nativeProject); + const nativeProject = quarantineInvalid + ? await this.#loadYaml(projectConfigPath) + : (this.#unwrapYamlLoadResult(projectConfigPath, await this.#loadYamlIfPresent(projectConfigPath, false)) ?? + {}); const nativeModelRoles = getByPath(nativeProject, ["modelRoles"]); if (nativeModelRoles !== undefined) { merged = this.#deepMerge(merged, { modelRoles: nativeModelRoles }); } - return this.#migrateRawSettings(merged); + return { + settings: this.#migrateRawSettings(merged, quarantineInvalid), + fileSettings: structuredClone(nativeProject), + shellPathSource, + }; + } + + async #loadProjectSettings(): Promise { + const result = await this.#readProjectSettings(true); + this.#projectFileSettings = result.fileSettings; + this.#projectShellPathSource = result.shellPathSource; + return result.settings; + } + + async #readConfigOverlays(captureLegacyChangelogVersion = true): Promise { + let shellPathSource: string | undefined; + let settings: RawSettings = {}; + for (const filePath of this.#configFiles) { + const overlay = await this.#loadOverlayYaml(filePath, captureLegacyChangelogVersion); + settings = this.#deepMerge(settings, overlay); + if (Object.hasOwn(overlay, "shellPath")) shellPathSource = filePath; + } + return { settings, shellPathSource }; } async #loadConfigOverlays(): Promise { - this.#overlayShellPathSource = undefined; - let merged: RawSettings = {}; - for (const filePath of this.#configFiles) { - const overlay = await this.#loadOverlayYaml(filePath); - merged = this.#deepMerge(merged, overlay); - if (Object.hasOwn(overlay, "shellPath")) this.#overlayShellPathSource = filePath; - } - return merged; + const result = await this.#readConfigOverlays(); + this.#overlayShellPathSource = result.shellPathSource; + return result.settings; } /** @@ -1271,7 +1400,7 @@ export class Settings { * missing or malformed files are hard errors so a typo'd path cannot * silently fall back to the persistent settings. */ - async #loadOverlayYaml(filePath: string): Promise { + async #loadOverlayYaml(filePath: string, captureLegacyChangelogVersion = true): Promise { let content: string; try { content = await Bun.file(filePath).text(); @@ -1292,7 +1421,7 @@ export class Settings { if (typeof parsed !== "object" || Array.isArray(parsed)) { throw new Error(`Config overlay must be a YAML mapping: ${filePath}`); } - return this.#migrateRawSettings(parsed as RawSettings); + return this.#migrateRawSettings(parsed as RawSettings, captureLegacyChangelogVersion); } async #migrateFromLegacy(): Promise { @@ -1333,7 +1462,7 @@ export class Settings { } /** Apply schema migrations to raw settings */ - #migrateRawSettings(raw: RawSettings): RawSettings { + #migrateRawSettings(raw: RawSettings, captureLegacyChangelogVersion = true): RawSettings { // queueMode -> steeringMode if ("queueMode" in raw && !("steeringMode" in raw)) { raw.steeringMode = raw.queueMode; @@ -1345,7 +1474,7 @@ export class Settings { // longer dirty user-tracked configs. Capture for marker seeding (see // #seedLastChangelogVersionMarker), then strip the key — the next // config save drops it from disk. - if (typeof raw.lastChangelogVersion === "string") { + if (captureLegacyChangelogVersion && typeof raw.lastChangelogVersion === "string") { this.#legacyLastChangelogVersion ??= raw.lastChangelogVersion; } delete raw.lastChangelogVersion; diff --git a/packages/coding-agent/src/task/structured-subagent.ts b/packages/coding-agent/src/task/structured-subagent.ts index 072e8abda..c57b0e9f5 100644 --- a/packages/coding-agent/src/task/structured-subagent.ts +++ b/packages/coding-agent/src/task/structured-subagent.ts @@ -245,6 +245,7 @@ function assertDepthAndSpawnAllowed(request: StructuredSubagentRequest, agentNam export async function resolveEffectiveSubagentPolicy( request: StructuredSubagentRequest, ): Promise { + await request.session.settings.reloadFromDisk(); const spawnPolicy = resolveSpawnPolicy(request.session.getSessionSpawns()); const agentName = request.agent?.trim() || spawnPolicy.defaultAgent; const planMode = request.session.getPlanModeState?.()?.enabled === true; diff --git a/packages/coding-agent/test/settings-manager.test.ts b/packages/coding-agent/test/settings-manager.test.ts index ca4833b62..e48bb4385 100644 --- a/packages/coding-agent/test/settings-manager.test.ts +++ b/packages/coding-agent/test/settings-manager.test.ts @@ -8,11 +8,13 @@ import { __providerInFlightForTesting, streamSimple } from "@oh-my-pi/pi-ai/stre import type { Context } from "@oh-my-pi/pi-ai/types"; import { onAppendOnlyModeChanged, + onModelRolesChanged, onStatusLineSessionAccentChanged, resetSettingsForTest, type SettingPath, Settings, } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as discovery from "@oh-my-pi/pi-coding-agent/discovery"; import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage"; import { AUTO_IMAGE_PROVIDER_ORDER } from "@oh-my-pi/pi-coding-agent/tools/image-providers"; import { SEARCH_PROVIDER_ORDER } from "@oh-my-pi/pi-coding-agent/web/search/types"; @@ -389,6 +391,90 @@ describe("Settings", () => { }); }); + describe("live persisted reload", () => { + it("rejects malformed live configs without moving them aside or replacing effective settings", async () => { + const projectConfigPath = path.join(projectDir, ".omp", "config.yml"); + await writeSettings({ + setupVersion: 1, + modelRoles: { global_role: "openai/global" }, + }); + await Bun.write( + projectConfigPath, + YAML.stringify({ modelRoles: { project_role: "openai/project" } }, null, 2), + ); + const settings = await Settings.init({ cwd: projectDir, agentDir }); + const malformedGlobal = 'setupVersion: 2\nmodelRoles:\n global_role: "unterminated\n'; + const malformedProject = 'modelRoles:\n project_role: "unterminated\n'; + await Promise.all([ + Bun.write(getConfigPath(), malformedGlobal), + Bun.write(projectConfigPath, malformedProject), + ]); + + await expect(settings.reloadFromDisk()).rejects.toThrow("Settings config is invalid"); + + expect(await Bun.file(getConfigPath()).text()).toBe(malformedGlobal); + expect(await Bun.file(projectConfigPath).text()).toBe(malformedProject); + expect(fs.readdirSync(agentDir).some(name => name.startsWith("config.yml.broken-"))).toBe(false); + expect( + fs.readdirSync(path.dirname(projectConfigPath)).some(name => name.startsWith("config.yml.broken-")), + ).toBe(false); + expect(settings.get("setupVersion")).toBe(1); + expect(settings.getModelRole("global_role")).toBe("openai/global"); + expect(settings.getModelRole("project_role")).toBe("openai/project"); + }); + it("retries when a persisted setting changes while files are being read", async () => { + await writeSettings({ setupVersion: 1 }); + const settings = await Settings.init({ cwd: projectDir, agentDir }); + const loadCapability = discovery.loadCapability; + const projectLoadStarted = Promise.withResolvers(); + const releaseProjectLoad = Promise.withResolvers(); + let pauseProjectLoad = true; + vi.spyOn(discovery, "loadCapability").mockImplementation(async (id, options) => { + if (pauseProjectLoad) { + pauseProjectLoad = false; + projectLoadStarted.resolve(); + await releaseProjectLoad.promise; + } + return await loadCapability(id, options); + }); + + const reload = settings.reloadFromDisk(); + await projectLoadStarted.promise; + settings.set("setupVersion", 2); + releaseProjectLoad.resolve(); + await reload; + await settings.flush(); + + expect(settings.get("setupVersion")).toBe(2); + expect((await readSettings()).setupVersion).toBe(2); + }); + + it("preserves runtime overrides and only signals semantic model-role changes", async () => { + await writeSettings({ modelRoles: { default: "openai/original" } }); + const settings = await Settings.init({ cwd: projectDir, agentDir }); + settings.overrideModelRoles({ runtime: "openai/runtime" }); + let signalCount = 0; + const unsubscribe = onModelRolesChanged(() => { + signalCount++; + }); + + try { + await settings.reloadFromDisk(); + expect(signalCount).toBe(0); + expect(settings.getModelRole("runtime")).toBe("openai/runtime"); + + await writeSettings({ modelRoles: { default: "openai/updated" } }); + await settings.reloadFromDisk(); + + expect(signalCount).toBe(1); + expect(settings.getModelRole("default")).toBe("openai/updated"); + expect(settings.getModelRole("runtime")).toBe("openai/runtime"); + } finally { + unsubscribe(); + } + }); + }); + describe("get()", () => { it("resolves overrides, schema defaults, and falsey values", () => { const isolated = Settings.isolated({ diff --git a/packages/coding-agent/test/task/structured-subagent.test.ts b/packages/coding-agent/test/task/structured-subagent.test.ts index 358d38a4d..235b670e2 100644 --- a/packages/coding-agent/test/task/structured-subagent.test.ts +++ b/packages/coding-agent/test/task/structured-subagent.test.ts @@ -167,6 +167,35 @@ describe("structured subagent primitive", () => { ).rejects.toThrow("isolation, apply, and merge controls are unavailable in plan mode"); expect(discover).not.toHaveBeenCalled(); }); + it("reloads model roles before resolving an agent added during the session", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-task-hot-reload-")); + const projectDir = path.join(root, "project"); + const agentDir = path.join(root, "agent"); + await fs.mkdir(projectDir, { recursive: true }); + const liveSettings = await Settings.loadIsolated({ cwd: projectDir, agentDir }); + const liveSession = { + ...session(), + cwd: projectDir, + settings: liveSettings, + } as ToolSession; + + try { + await Bun.write(path.join(projectDir, ".omp", "config.yml"), "modelRoles:\n hot_worker: kimi-code/k3:max\n"); + await Bun.write( + path.join(projectDir, ".omp", "agents", "hot-worker.md"), + '---\nname: hot-worker\ndescription: Newly added worker.\nmodel: "@hot_worker"\n---\n\nInspect the assignment.\n', + ); + + const policy = await resolveEffectiveSubagentPolicy(request({ session: liveSession, agent: "hot-worker" })); + + expect(policy.modelRole).toBe("hot_worker"); + expect(policy.modelOverride).toEqual(["kimi-code/k3:max"]); + } finally { + liveSettings.cancelPendingSaves(); + await fs.rm(root, { recursive: true, force: true }); + } + }); + it("propagates a custom thinking-suffixed role alias through policy, dispatch, and settlement", async () => { const customAgent = { ...AGENT, model: ["@reviewer:high"] }; mockDiscovery(customAgent);