fix(vscode): preserve agent override clear semantics

This commit is contained in:
Santiago Sainz
2026-03-27 21:47:41 -06:00
parent 85ecae8f6f
commit 2c547be050
12 changed files with 310 additions and 73 deletions
+29 -20
View File
@@ -1910,32 +1910,41 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
// races with the async config.update() write on the CLI backend).
this.pending++
try {
await this.client.global.config.update({ config: partial }, { throwOnError: true })
try {
await this.client.global.config.update({ config: partial }, { throwOnError: true })
} catch (error) {
console.error("[Kilo New] KiloProvider: Failed to update config:", error)
this.postMessage({
type: "error",
message: getErrorMessage(error) || "Failed to update config",
})
this.postMessage({
type: "configSaveFailed",
config: (this.cachedConfigMessage as { config: unknown } | undefined)?.config ?? {},
})
return
}
// Re-fetch the full merged config (global + project + all layers) so the
// webview receives the complete resolved config, not just global-only data.
// Config.state is reset by updateGlobal (via Instance.resetStateEntry) so
// config.get() returns fresh data without a full dispose cycle.
const dir = this.getWorkspaceDirectory()
const { data: merged } = await this.client.config.get({ directory: dir }, { throwOnError: true })
try {
// Re-fetch the full merged config (global + project + all layers) so the
// webview receives the complete resolved config, not just global-only data.
// Config.state is reset by updateGlobal (via Instance.resetStateEntry) so
// config.get() returns fresh data without a full dispose cycle.
const dir = this.getWorkspaceDirectory()
const { data: merged } = await this.client.config.get({ directory: dir }, { throwOnError: true })
this.cachedConfigMessage = { type: "configLoaded", config: merged }
this.postMessage({ type: "configUpdated", config: merged })
this.cachedConfigMessage = { type: "configLoaded", config: merged }
this.postMessage({ type: "configUpdated", config: merged })
} catch (error) {
console.error("[Kilo New] KiloProvider: Failed to fetch merged config after update:", error)
// The write already succeeded; keep the optimistic state and wait for
// the SSE-driven refresh to deliver the authoritative merged config.
this.postMessage({ type: "configSaved" })
}
if (refreshProviders) {
await this.fetchAndSendProviders()
}
} catch (error) {
console.error("[Kilo New] KiloProvider: Failed to update config:", error)
this.postMessage({
type: "error",
message: getErrorMessage(error) || "Failed to update config",
})
// Send configUpdated with the last known good config so the webview
// clears its saving flag and reverts optimistic state.
if (this.cachedConfigMessage) {
this.postMessage({ type: "configUpdated", config: (this.cachedConfigMessage as { config: unknown }).config })
}
} finally {
this.pending--
}
@@ -0,0 +1,55 @@
import { describe, expect, it } from "bun:test"
import {
selectedAgentNumberOverrideValue,
selectedAgentTextOverrideValue,
selectedDefaultAgentValue,
shouldClearDefaultAgentWhenAgentBecomesUnavailable,
} from "../../webview-ui/src/components/settings/agent-behaviour-patches"
describe("selectedAgentTextOverrideValue", () => {
it("maps an empty text field value to a null delete sentinel", () => {
expect(selectedAgentTextOverrideValue("")).toBeNull()
})
it("preserves a non-empty text override", () => {
expect(selectedAgentTextOverrideValue("Review code")).toBe("Review code")
})
})
describe("selectedAgentNumberOverrideValue", () => {
it("maps a blank numeric field value to a null delete sentinel", () => {
expect(selectedAgentNumberOverrideValue("", parseFloat)).toBeNull()
})
it("preserves a valid numeric override", () => {
expect(selectedAgentNumberOverrideValue("0.7", parseFloat)).toBe(0.7)
})
it("keeps invalid non-empty numeric input out of the persisted patch", () => {
expect(selectedAgentNumberOverrideValue("abc", parseFloat)).toBeUndefined()
})
})
describe("selectedDefaultAgentValue", () => {
it("maps an empty dropdown value to a null delete sentinel", () => {
expect(selectedDefaultAgentValue("")).toBeNull()
})
it("preserves a non-empty agent selection", () => {
expect(selectedDefaultAgentValue("code")).toBe("code")
})
})
describe("shouldClearDefaultAgentWhenAgentBecomesUnavailable", () => {
it("clears when the current default agent becomes unavailable", () => {
expect(shouldClearDefaultAgentWhenAgentBecomesUnavailable(true, "code", "code")).toBe(true)
})
it("does not clear when toggling a non-default agent", () => {
expect(shouldClearDefaultAgentWhenAgentBecomesUnavailable(true, "code", "plan")).toBe(false)
})
it("does not clear when the agent remains available", () => {
expect(shouldClearDefaultAgentWhenAgentBecomesUnavailable(false, "code", "code")).toBe(false)
})
})
@@ -189,6 +189,71 @@ describe("ConfigState", () => {
expect(s.saving).toBe(false)
expect(Object.keys(s.draft).length).toBe(0)
})
it("preserves the null delete sentinel in the pending save payload", () => {
const s = new ConfigState()
s.handleConfigLoaded({ default_agent: "code" })
s.updateConfig({ default_agent: null })
expect(s.draft.default_agent).toBeNull()
s.saveConfig()
expect(s.saving).toBe(true)
expect(s.draft.default_agent).toBeNull()
})
})
describe("configSaved while a save is in-flight", () => {
it("clears the draft after a confirmed write even if merged refresh is pending", () => {
const s = new ConfigState()
s.handleConfigLoaded({ agent: { code: { prompt: "Review" } } })
s.updateConfig({ agent: { code: { prompt: null } } })
s.saveConfig()
s.handleConfigSaved()
expect(s.saving).toBe(false)
expect(s.dirty).toBe(false)
expect(Object.keys(s.draft).length).toBe(0)
expect(s.saved.agent?.code?.prompt).toBeUndefined()
expect(s.config.agent?.code?.prompt).toBeUndefined()
})
})
describe("configSaveFailed while a save is in-flight", () => {
it("preserves pending null-sentinel clears so the user can retry", () => {
const s = new ConfigState()
s.handleConfigLoaded({ agent: { code: { prompt: "Review", temperature: 0.7 } }, default_agent: "code" })
s.updateConfig({ agent: { code: { prompt: null, temperature: null } } })
s.updateConfig({ default_agent: null })
s.saveConfig()
s.handleConfigSaveFailed({ agent: { code: { prompt: "Review", temperature: 0.7 } }, default_agent: "code" })
expect(s.saving).toBe(false)
expect(s.dirty).toBe(true)
expect(s.draft.agent?.code?.prompt).toBeNull()
expect(s.draft.agent?.code?.temperature).toBeNull()
expect(s.draft.default_agent).toBeNull()
expect(s.config.agent?.code?.prompt).toBeUndefined()
expect(s.config.agent?.code?.temperature).toBeUndefined()
expect(s.config.default_agent).toBeUndefined()
})
})
it("ignores repeated save attempts while a save is already in-flight", () => {
const s = new ConfigState()
s.handleConfigLoaded({ snapshot: true })
s.updateConfig({ snapshot: false })
s.saveConfig()
s.saveConfig()
s.handleConfigUpdated({ snapshot: false })
expect(s.saving).toBe(false)
expect(s.dirty).toBe(false)
expect(s.config.snapshot).toBe(false)
})
it("configLoaded is ignored while save is in-flight", () => {
@@ -15,6 +15,7 @@ import ModeEditView from "./ModeEditView"
import ModeCreateView from "./ModeCreateView"
import McpEditView from "./McpEditView"
import WorkflowsTab from "./agent-behaviour/WorkflowsTab"
import { selectedDefaultAgentValue } from "./agent-behaviour-patches"
import { parseImport, MAX_IMPORT_SIZE } from "./mode-io"
import type { ImportError } from "./mode-io"
@@ -274,7 +275,7 @@ const AgentBehaviourTab: Component = () => {
label={(o) => o.label}
onSelect={(o) => {
if (!o) return
const next = o.value || null
const next = selectedDefaultAgentValue(o.value)
if (next === (config().default_agent ?? null)) return
updateConfig({ default_agent: next })
}}
@@ -10,6 +10,11 @@ import { useSession } from "../../context/session"
import { useLanguage } from "../../context/language"
import type { AgentConfig, AgentInfo } from "../../types/messages"
import SettingsRow from "./SettingsRow"
import {
selectedAgentNumberOverrideValue,
selectedAgentTextOverrideValue,
shouldClearDefaultAgentWhenAgentBecomesUnavailable,
} from "./agent-behaviour-patches"
import { buildExport } from "./mode-io"
interface Props {
@@ -42,6 +47,16 @@ const ModeEditView: Component<Props> = (props) => {
})
}
const updateNumberOverride = (
key: "temperature" | "top_p" | "steps",
rawValue: string,
parse: (value: string) => number,
) => {
const next = selectedAgentNumberOverrideValue(rawValue, parse)
if (rawValue.trim() !== "" && next === undefined) return
update({ [key]: next } as Pick<AgentConfig, typeof key>)
}
const exportMode = () => {
const data = buildExport(props.name, cfg())
const json = JSON.stringify(data, null, 2)
@@ -106,19 +121,16 @@ const ModeEditView: Component<Props> = (props) => {
</Card>
</Show>
{/* Description (full-width, custom modes only) */}
<Show when={!native()}>
<Card style={{ "margin-bottom": "12px" }}>
<div data-slot="settings-row-label-title" style={{ "margin-bottom": "8px" }}>
{language.t("settings.agentBehaviour.editMode.description")}
</div>
<TextField
value={cfg().description ?? ""}
placeholder={language.t("settings.agentBehaviour.createMode.description.placeholder")}
onChange={(val) => update({ description: val || undefined })}
/>
</Card>
</Show>
<Card style={{ "margin-bottom": "12px" }}>
<div data-slot="settings-row-label-title" style={{ "margin-bottom": "8px" }}>
{language.t("settings.agentBehaviour.editMode.description")}
</div>
<TextField
value={cfg().description ?? ""}
placeholder={language.t("settings.agentBehaviour.createMode.description.placeholder")}
onChange={(val) => update({ description: selectedAgentTextOverrideValue(val) })}
/>
</Card>
{/* Prompt (full-width, auto-resizing) */}
<Card style={{ "margin-bottom": "12px" }}>
@@ -131,7 +143,7 @@ const ModeEditView: Component<Props> = (props) => {
value={cfg().prompt ?? ""}
placeholder={language.t("settings.agentBehaviour.createMode.prompt.placeholder")}
multiline
onChange={(val) => update({ prompt: val || undefined })}
onChange={(val) => update({ prompt: selectedAgentTextOverrideValue(val) })}
/>
</Card>
@@ -144,7 +156,7 @@ const ModeEditView: Component<Props> = (props) => {
<TextField
value={cfg().model ?? ""}
placeholder="e.g. anthropic/claude-sonnet-4-20250514"
onChange={(val) => update({ model: val || undefined })}
onChange={(val) => update({ model: selectedAgentTextOverrideValue(val) })}
/>
</SettingsRow>
@@ -155,10 +167,7 @@ const ModeEditView: Component<Props> = (props) => {
<TextField
value={cfg().temperature?.toString() ?? ""}
placeholder={language.t("common.default")}
onChange={(val) => {
const parsed = parseFloat(val)
update({ temperature: isNaN(parsed) ? undefined : parsed })
}}
onChange={(val) => updateNumberOverride("temperature", val, parseFloat)}
/>
</SettingsRow>
@@ -169,10 +178,7 @@ const ModeEditView: Component<Props> = (props) => {
<TextField
value={cfg().top_p?.toString() ?? ""}
placeholder={language.t("common.default")}
onChange={(val) => {
const parsed = parseFloat(val)
update({ top_p: isNaN(parsed) ? undefined : parsed })
}}
onChange={(val) => updateNumberOverride("top_p", val, parseFloat)}
/>
</SettingsRow>
@@ -183,10 +189,7 @@ const ModeEditView: Component<Props> = (props) => {
<TextField
value={cfg().steps?.toString() ?? ""}
placeholder={language.t("common.default")}
onChange={(val) => {
const parsed = parseInt(val, 10)
update({ steps: isNaN(parsed) ? undefined : parsed })
}}
onChange={(val) => updateNumberOverride("steps", val, (raw) => parseInt(raw, 10))}
/>
</SettingsRow>
@@ -199,7 +202,7 @@ const ModeEditView: Component<Props> = (props) => {
onChange={(val) => {
update({ hidden: val })
// Clear default_agent if hiding the current default
if (val && config().default_agent === props.name) {
if (shouldClearDefaultAgentWhenAgentBecomesUnavailable(val, config().default_agent, props.name)) {
updateConfig({ default_agent: null })
}
}}
@@ -219,7 +222,7 @@ const ModeEditView: Component<Props> = (props) => {
onChange={(val) => {
update({ disable: val })
// Clear default_agent if disabling the current default
if (val && config().default_agent === props.name) {
if (shouldClearDefaultAgentWhenAgentBecomesUnavailable(val, config().default_agent, props.name)) {
updateConfig({ default_agent: null })
}
}}
@@ -0,0 +1,24 @@
export function selectedDefaultAgentValue(value: string): string | null {
return value || null
}
export function selectedAgentTextOverrideValue(value: string): string | null {
return value === "" ? null : value
}
export function selectedAgentNumberOverrideValue(
value: string,
parse: (value: string) => number,
): number | null | undefined {
if (value.trim() === "") return null
const parsed = parse(value)
return Number.isNaN(parsed) ? undefined : parsed
}
export function shouldClearDefaultAgentWhenAgentBecomesUnavailable(
nextValue: boolean,
currentDefaultAgent: string | null | undefined,
agentName: string,
): boolean {
return nextValue && currentDefaultAgent === agentName
}
@@ -67,6 +67,22 @@ export const ConfigProvider: ParentComponent = (props) => {
setSaved(message.config)
return
}
if (message.type === "configSaved") {
if (!saving) return
const confirmedConfig = config()
saving = false
setDraft({})
setIsDirty(false)
setSaved(confirmedConfig)
return
}
if (message.type === "configSaveFailed") {
if (!saving) return
saving = false
setSaved(message.config)
setConfig(resolveConfig(message.config, draft(), isDirty()))
return
}
})
onCleanup(unsubscribe)
@@ -101,7 +117,7 @@ export const ConfigProvider: ParentComponent = (props) => {
function saveConfig() {
const changes = draft()
if (Object.keys(changes).length === 0) return
if (saving || Object.keys(changes).length === 0) return
// Don't clear draft/isDirty yet — wait for configUpdated confirmation.
// If the write fails, the save bar stays visible so the user can retry.
saving = true
@@ -321,14 +321,14 @@ export type PermissionConfig = Partial<Record<string, PermissionRule>>
export interface AgentConfig {
model?: string | null
prompt?: string
description?: string
prompt?: string | null
description?: string | null
mode?: "subagent" | "primary" | "all"
hidden?: boolean
disable?: boolean
temperature?: number
top_p?: number
steps?: number
temperature?: number | null
top_p?: number | null
steps?: number | null
permission?: PermissionConfig
}
@@ -722,6 +722,15 @@ export interface ConfigUpdatedMessage {
config: Config
}
export interface ConfigSavedMessage {
type: "configSaved"
}
export interface ConfigSaveFailedMessage {
type: "configSaveFailed"
config: Config
}
export interface GlobalConfigLoadedMessage {
type: "globalConfigLoaded"
config: Config
@@ -1277,6 +1286,8 @@ export type ExtensionMessage =
| BrowserSettingsLoadedMessage
| ConfigLoadedMessage
| ConfigUpdatedMessage
| ConfigSavedMessage
| ConfigSaveFailedMessage
| GlobalConfigLoadedMessage
| NotificationSettingsLoadedMessage
| NotificationsLoadedMessage
@@ -81,9 +81,26 @@ export class ConfigState {
this.saved = server
}
/** Handle a confirmed save when merged config refresh is still pending. */
handleConfigSaved() {
if (!this.saving) return
this.saving = false
this.draft = {}
this.dirty = false
this.saved = this.config
}
/** Handle an explicit save failure from the extension. */
handleConfigSaveFailed(server: Config) {
if (!this.saving) return
this.saving = false
this.saved = server
this.config = resolveConfig(server, this.draft, this.dirty)
}
/** Send the draft to the backend. */
saveConfig() {
if (Object.keys(this.draft).length === 0) return
if (this.saving || Object.keys(this.draft).length === 0) return
this.saving = true
}
+5 -4
View File
@@ -819,12 +819,12 @@ export namespace Config {
.string()
.optional()
.describe("Default model variant for this agent (applies only when using the agent's configured model)."),
temperature: z.number().optional(),
top_p: z.number().optional(),
prompt: z.string().optional(),
temperature: z.number().nullable().optional(), // kilocode_change - nullable for delete sentinel
top_p: z.number().nullable().optional(), // kilocode_change - nullable for delete sentinel
prompt: z.string().nullable().optional(), // kilocode_change - nullable for delete sentinel
tools: z.record(z.string(), z.boolean()).optional().describe("@deprecated Use 'permission' field instead"),
disable: z.boolean().optional(),
description: z.string().optional().describe("Description of when to use the agent"),
description: z.string().nullable().optional().describe("Description of when to use the agent"),
mode: z.enum(["subagent", "primary", "all"]).optional(),
hidden: z
.boolean()
@@ -842,6 +842,7 @@ export namespace Config {
.number()
.int()
.positive()
.nullable()
.optional()
.describe("Maximum number of agentic iterations before forcing text-only response"),
maxSteps: z.number().int().positive().optional().describe("@deprecated Use 'steps' field instead."),
+5 -5
View File
@@ -1113,9 +1113,9 @@ export type AgentConfig = {
* Default model variant for this agent (applies only when using the agent's configured model).
*/
variant?: string
temperature?: number
top_p?: number
prompt?: string
temperature?: number | null
top_p?: number | null
prompt?: string | null
/**
* @deprecated Use 'permission' field instead
*/
@@ -1126,7 +1126,7 @@ export type AgentConfig = {
/**
* Description of when to use the agent
*/
description?: string
description?: string | null
mode?: "subagent" | "primary" | "all"
/**
* Hide this subagent from the @ autocomplete menu (default: false, only applies to mode: subagent)
@@ -1142,7 +1142,7 @@ export type AgentConfig = {
/**
* Maximum number of agentic iterations before forcing text-only response
*/
steps?: number
steps?: number | null
/**
* @deprecated Use 'steps' field instead.
*/
+42 -7
View File
@@ -12387,13 +12387,34 @@
"type": "string"
},
"temperature": {
"type": "number"
"anyOf": [
{
"type": "number"
},
{
"type": "null"
}
]
},
"top_p": {
"type": "number"
"anyOf": [
{
"type": "number"
},
{
"type": "null"
}
]
},
"prompt": {
"type": "string"
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
]
},
"tools": {
"description": "@deprecated Use 'permission' field instead",
@@ -12410,7 +12431,14 @@
},
"description": {
"description": "Description of when to use the agent",
"type": "string"
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
]
},
"mode": {
"type": "string",
@@ -12454,9 +12482,16 @@
},
"steps": {
"description": "Maximum number of agentic iterations before forcing text-only response",
"type": "integer",
"exclusiveMinimum": 0,
"maximum": 9007199254740991
"anyOf": [
{
"type": "integer",
"exclusiveMinimum": 0,
"maximum": 9007199254740991
},
{
"type": "null"
}
]
},
"maxSteps": {
"description": "@deprecated Use 'steps' field instead.",