mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-10-29 18:52:45 +08:00
fix(vscode): speed up settings saves
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"kilo-code": patch
|
||||
"@kilocode/cli": patch
|
||||
---
|
||||
|
||||
Speed up VS Code settings saves by draining and disposing worktree instances concurrently, then finishing once config writes succeed.
|
||||
@@ -59,7 +59,7 @@ import type { RemoteStatusService } from "./services/RemoteStatusService"
|
||||
import { resolveProjectDirectory } from "./project-directory"
|
||||
import { seedSessionStatuses } from "./session-status"
|
||||
import { normalizeEnhancePromptErrorMessage } from "./enhance-prompt-error"
|
||||
import { retry } from "./services/cli-backend/retry"
|
||||
import { deadline, retry } from "./services/cli-backend/retry"
|
||||
import { slimInfo, slimPart, slimParts } from "./kilo-provider/slim-metadata"
|
||||
import { handleSidebarWorktreeMessage } from "./kilo-provider/sidebar-worktree"
|
||||
import { parseMessageFiles, type MessageFile } from "./kilo-provider/message-files"
|
||||
@@ -361,6 +361,8 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
private cachedMcpStatusMessage: unknown = null
|
||||
/** Ref-count of in-flight handleUpdateConfig calls; prevents fetchAndSendConfig from sending stale data */
|
||||
private pending = 0
|
||||
private configRevision = 0
|
||||
private refreshWait = 5_000
|
||||
private configWarningsShown = false
|
||||
/** Cached notificationsLoaded payload */
|
||||
private cachedNotificationsMessage: NotificationsMessage | null = null
|
||||
@@ -2432,11 +2434,13 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
|
||||
try {
|
||||
const workspaceDir = this.getWorkspaceDirectory()
|
||||
const revision = this.configRevision
|
||||
const [{ data: config }, { data: global }, { data: overlay }] = await Promise.all([
|
||||
retry(() => this.client!.config.get({ directory: workspaceDir }, { throwOnError: true })),
|
||||
this.client.global.config.get({ throwOnError: true }),
|
||||
this.client.config.overlay({ directory: workspaceDir, scope: "project" }, { throwOnError: true }),
|
||||
])
|
||||
if (revision !== this.configRevision) return
|
||||
this.cachedGlobalConfig = global ?? null
|
||||
|
||||
const message = {
|
||||
@@ -2457,8 +2461,10 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
/** Fetch global-only config (no project/managed layers) for settings export. */
|
||||
private async fetchAndSendGlobalConfig(): Promise<void> {
|
||||
if (!this.client || this.connectionState !== "connected") return
|
||||
const revision = this.configRevision
|
||||
try {
|
||||
const { data: config } = await this.client.global.config.get({ throwOnError: true })
|
||||
if (revision !== this.configRevision) return
|
||||
this.cachedGlobalConfig = config ?? null
|
||||
this.postMessage({ type: "globalConfigLoaded", config })
|
||||
} catch (error) {
|
||||
@@ -2541,6 +2547,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
*/
|
||||
private async fetchAndSendConfigUpdated(): Promise<void> {
|
||||
if (!this.client || this.connectionState !== "connected") return
|
||||
const revision = ++this.configRevision
|
||||
try {
|
||||
const dir = this.getWorkspaceDirectory()
|
||||
const [{ data: config }, { data: global }, { data: overlay }] = await Promise.all([
|
||||
@@ -2548,6 +2555,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
this.client.global.config.get({ throwOnError: true }),
|
||||
this.client.config.overlay({ directory: dir, scope: "project" }, { throwOnError: true }),
|
||||
])
|
||||
if (revision !== this.configRevision) return
|
||||
this.cachedGlobalConfig = global ?? null
|
||||
this.cachedConfigMessage = {
|
||||
type: "configLoaded",
|
||||
@@ -2910,6 +2918,7 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
const hasProject = Object.keys(project).length > 0 || projectUnset.length > 0
|
||||
|
||||
this.pending++
|
||||
this.configRevision++
|
||||
const dir = this.getWorkspaceDirectory()
|
||||
|
||||
try {
|
||||
@@ -2927,17 +2936,32 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
)
|
||||
}
|
||||
} catch (error) {
|
||||
this.postConfigFailure(error)
|
||||
this.cachedConfigMessage = null
|
||||
this.cachedGlobalConfig = null
|
||||
this.pending--
|
||||
await deadline(
|
||||
this.fetchAndSendConfigUpdated(),
|
||||
this.refreshWait,
|
||||
"Timed out refreshing config after a failed save",
|
||||
).catch((err) => console.error("[Kilo New] KiloProvider: Failed to refresh config after a failed save:", err))
|
||||
this.postConfigFailure(error)
|
||||
return
|
||||
}
|
||||
|
||||
const revision = ++this.configRevision
|
||||
this.postMessage({ type: "configSaved" })
|
||||
|
||||
try {
|
||||
const [{ data: merged }, { data: global }, { data: overlay }] = await Promise.all([
|
||||
retry(() => this.client!.config.get({ directory: dir }, { throwOnError: true })),
|
||||
this.client.global.config.get({ throwOnError: true }),
|
||||
this.client.config.overlay({ directory: dir, scope: "project" }, { throwOnError: true }),
|
||||
])
|
||||
const [{ data: merged }, { data: global }, { data: overlay }] = await deadline(
|
||||
Promise.all([
|
||||
retry(() => this.client!.config.get({ directory: dir }, { throwOnError: true })),
|
||||
this.client.global.config.get({ throwOnError: true }),
|
||||
this.client.config.overlay({ directory: dir, scope: "project" }, { throwOnError: true }),
|
||||
]),
|
||||
this.refreshWait,
|
||||
"Timed out refreshing saved config",
|
||||
)
|
||||
if (revision !== this.configRevision) return
|
||||
this.cachedGlobalConfig = global ?? null
|
||||
this.cachedConfigMessage = {
|
||||
type: "configLoaded",
|
||||
@@ -2962,21 +2986,12 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper
|
||||
])
|
||||
} catch (error) {
|
||||
console.error("[Kilo New] KiloProvider: Config write succeeded but post-write refresh failed:", error)
|
||||
const patch =
|
||||
partial.indexing === undefined && project.indexing === undefined
|
||||
? { ...partial, ...project }
|
||||
: { ...partial, ...project, indexing: { ...(partial.indexing ?? {}), ...(project.indexing ?? {}) } }
|
||||
const cached = (this.cachedConfigMessage as { config?: unknown } | null)?.config
|
||||
const features = (this.cachedConfigMessage as { features?: unknown } | null)?.features
|
||||
const optimistic =
|
||||
cached && typeof cached === "object" ? { ...(cached as Record<string, unknown>), ...patch } : patch
|
||||
this.postMessage({
|
||||
type: "configUpdated",
|
||||
config: optimistic,
|
||||
globalConfig: this.cachedGlobalConfig ?? undefined,
|
||||
settings: { maxCost: this.maxCostSetting(), languageCommitMessage: this.commitMessageLanguageSetting() },
|
||||
features: features ?? configFeatures(optimistic as Config),
|
||||
})
|
||||
// The webview already holds the optimistic state acknowledged by configSaved.
|
||||
// Never replay stale scoped config when the authoritative refresh is unavailable.
|
||||
if (revision === this.configRevision) {
|
||||
this.cachedConfigMessage = null
|
||||
this.cachedGlobalConfig = null
|
||||
}
|
||||
this.requirements.clear()
|
||||
} finally {
|
||||
this.pending--
|
||||
|
||||
@@ -110,4 +110,108 @@ describe("KiloConnectionService drainPendingPrompts", () => {
|
||||
|
||||
await expect(service.drainPendingPrompts()).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
test("drains four directories concurrently and suggestions once", async () => {
|
||||
const service = new KiloConnectionService({} as any)
|
||||
const dirs = ["/tmp/a", "/tmp/b", "/tmp/c", "/tmp/d", "/tmp/e"]
|
||||
const gates = new Map(dirs.map((dir) => [dir, Promise.withResolvers<void>()]))
|
||||
const fifth = Promise.withResolvers<void>()
|
||||
const calls: string[] = []
|
||||
let cleared = 0
|
||||
const client = {
|
||||
permission: {
|
||||
list: async ({ directory }: { directory: string }) => {
|
||||
calls.push(`permission:${directory}`)
|
||||
if (directory === dirs[4]) fifth.resolve()
|
||||
await gates.get(directory)!.promise
|
||||
return { data: [] }
|
||||
},
|
||||
},
|
||||
question: {
|
||||
list: async ({ directory }: { directory: string }) => {
|
||||
calls.push(`question:${directory}`)
|
||||
return { data: [] }
|
||||
},
|
||||
},
|
||||
suggestion: {
|
||||
list: async ({ directory }: { directory: string }) => {
|
||||
calls.push(`suggestion:${directory}`)
|
||||
return { data: [] }
|
||||
},
|
||||
},
|
||||
network: {
|
||||
list: async ({ directory }: { directory: string }) => {
|
||||
calls.push(`network:${directory}`)
|
||||
return { data: [] }
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
;(service as any).client = client
|
||||
;(service as any).directoryProviders.add(() => dirs)
|
||||
service.onClearPendingPrompts(() => cleared++)
|
||||
|
||||
const pending = service.drainPendingPrompts()
|
||||
expect(calls).toEqual(dirs.slice(0, 4).map((dir) => `permission:${dir}`))
|
||||
|
||||
gates.get(dirs[0])!.resolve()
|
||||
await fifth.promise
|
||||
expect(calls.filter((call) => call.startsWith("permission:"))).toEqual(dirs.map((dir) => `permission:${dir}`))
|
||||
|
||||
for (const gate of gates.values()) gate.resolve()
|
||||
await pending
|
||||
|
||||
expect(calls.filter((call) => call.startsWith("suggestion:"))).toEqual([`suggestion:${dirs[0]}`])
|
||||
const suggestion = calls.findIndex((call) => call.startsWith("suggestion:"))
|
||||
expect(calls.filter((call) => call.startsWith("question:")).every((call) => calls.indexOf(call) < suggestion)).toBe(
|
||||
true,
|
||||
)
|
||||
expect(calls.filter((call) => call.startsWith("network:")).every((call) => calls.indexOf(call) > suggestion)).toBe(
|
||||
true,
|
||||
)
|
||||
expect(cleared).toBe(1)
|
||||
})
|
||||
|
||||
test("waits for active drains and skips queued directories after a failure", async () => {
|
||||
const service = new KiloConnectionService({} as any)
|
||||
const dirs = ["/tmp/a", "/tmp/b", "/tmp/c", "/tmp/d", "/tmp/e"]
|
||||
const release = Promise.withResolvers<void>()
|
||||
const calls: string[] = []
|
||||
let cleared = 0
|
||||
const client = {
|
||||
permission: {
|
||||
list: async ({ directory }: { directory: string }) => {
|
||||
calls.push(directory)
|
||||
if (directory === dirs[0]) await release.promise
|
||||
if (directory === dirs[1]) return { error: "failed" }
|
||||
return { data: [] }
|
||||
},
|
||||
},
|
||||
question: { list: async () => ({ data: [] }) },
|
||||
suggestion: { list: async () => ({ data: [] }) },
|
||||
network: { list: async () => ({ data: [] }) },
|
||||
}
|
||||
|
||||
;(service as any).client = client
|
||||
;(service as any).directoryProviders.add(() => dirs)
|
||||
service.onClearPendingPrompts(() => cleared++)
|
||||
|
||||
const pending = service.drainPendingPrompts()
|
||||
expect(calls).toEqual(dirs.slice(0, 4))
|
||||
expect(
|
||||
await Promise.race([
|
||||
pending.then(
|
||||
() => "settled",
|
||||
() => "settled",
|
||||
),
|
||||
Promise.resolve("pending"),
|
||||
]),
|
||||
).toBe("pending")
|
||||
expect(calls).not.toContain(dirs[4])
|
||||
|
||||
release.resolve()
|
||||
await expect(pending).rejects.toThrow(`Failed to list permissions for ${dirs[1]}`)
|
||||
expect(calls).not.toContain(dirs[4])
|
||||
expect(cleared).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -18,6 +18,28 @@ type FavoritesChangeListener = (favorites: Array<{ providerID: string; modelID:
|
||||
type ModelSelectorExpandedListener = (value: boolean) => void
|
||||
type ClearPendingPromptsListener = () => void
|
||||
type DirectoryProvider = () => string[]
|
||||
const DRAIN_CONCURRENCY = 4
|
||||
|
||||
async function parallel<T>(items: T[], fn: (item: T) => Promise<void>): Promise<void> {
|
||||
let next = 0
|
||||
const errors = new Map<number, unknown>()
|
||||
const worker = async () => {
|
||||
while (errors.size === 0) {
|
||||
const index = next++
|
||||
if (index >= items.length) return
|
||||
try {
|
||||
await fn(items[index]!)
|
||||
} catch (error) {
|
||||
errors.set(index, error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await Promise.all(Array.from({ length: Math.min(DRAIN_CONCURRENCY, items.length) }, worker))
|
||||
if (errors.size === 0) return
|
||||
const index = Math.min(...errors.keys())
|
||||
throw errors.get(index)
|
||||
}
|
||||
|
||||
function isNotFound(err: unknown) {
|
||||
if (!err || typeof err !== "object") return false
|
||||
@@ -505,7 +527,8 @@ export class KiloConnectionService {
|
||||
* destructive operation.
|
||||
*/
|
||||
async drainPendingPrompts(): Promise<void> {
|
||||
if (!this.client) return
|
||||
const client = this.client
|
||||
if (!client) return
|
||||
|
||||
// Only drain directories from currently-mounted providers (root + worktree dirs).
|
||||
// Previously this also called project.list() to include every historically-opened
|
||||
@@ -519,26 +542,29 @@ export class KiloConnectionService {
|
||||
}
|
||||
}
|
||||
|
||||
for (const dir of dirs) {
|
||||
const { data: perms, error: permsErr } = await this.client.permission.list({ directory: dir })
|
||||
const list = [...dirs]
|
||||
await parallel(list, async (dir) => {
|
||||
const { data: perms, error: permsErr } = await client.permission.list({ directory: dir })
|
||||
if (permsErr) throw new Error(`Failed to list permissions for ${dir}: ${String(permsErr)}`)
|
||||
if (perms) {
|
||||
for (const perm of perms) {
|
||||
const { error } = await this.client.permission.reply({ requestID: perm.id, reply: "reject", directory: dir })
|
||||
const { error } = await client.permission.reply({ requestID: perm.id, reply: "reject", directory: dir })
|
||||
if (error && !isNotFound(error)) throw new Error(`Failed to reject permission ${perm.id}: ${String(error)}`)
|
||||
}
|
||||
}
|
||||
const { data: qs, error: qsErr } = await this.client.question.list({ directory: dir })
|
||||
const { data: qs, error: qsErr } = await client.question.list({ directory: dir })
|
||||
if (qsErr) throw new Error(`Failed to list questions for ${dir}: ${String(qsErr)}`)
|
||||
if (qs) {
|
||||
for (const q of qs) {
|
||||
const { error } = await this.client.question.reject({ requestID: q.id, directory: dir })
|
||||
const { error } = await client.question.reject({ requestID: q.id, directory: dir })
|
||||
if (error && !isNotFound(error)) throw new Error(`Failed to reject question ${q.id}: ${String(error)}`)
|
||||
}
|
||||
}
|
||||
await drainSuggestions(this.client, dir)
|
||||
await drainNetworkWaits(this.client, dir)
|
||||
}
|
||||
})
|
||||
|
||||
// Suggestions are backend-global despite the directory-bearing SDK route.
|
||||
if (list[0]) await drainSuggestions(client, list[0])
|
||||
await parallel(list, (dir) => drainNetworkWaits(client, dir))
|
||||
for (const listener of this.clearPendingPromptsListeners) {
|
||||
listener()
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// Replicated from packages/core/src/util/retry.ts to avoid adding @opencode-ai/core
|
||||
// as a dependency of the extension. Keep in sync with the original.
|
||||
// retry() is replicated from packages/core/src/util/retry.ts to avoid adding @opencode-ai/core
|
||||
// as a dependency of the extension. Keep retry() in sync with the original.
|
||||
|
||||
const TRANSIENT = [
|
||||
"load failed",
|
||||
@@ -32,3 +32,19 @@ export async function retry<T>(fn: () => Promise<T>, attempts = 3, delay = 500):
|
||||
}
|
||||
throw last
|
||||
}
|
||||
|
||||
export function deadline<T>(task: Promise<T>, delay: number, message: string): Promise<T> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => reject(new Error(message)), delay)
|
||||
task.then(
|
||||
(value) => {
|
||||
clearTimeout(timer)
|
||||
resolve(value)
|
||||
},
|
||||
(err) => {
|
||||
clearTimeout(timer)
|
||||
reject(err)
|
||||
},
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -198,17 +198,22 @@ describe("ConfigState", () => {
|
||||
expect(s.config.username).toBe("bob") // server update applied
|
||||
})
|
||||
|
||||
it("clears draft when update confirms our save", () => {
|
||||
it("waits for explicit save confirmation before clearing the draft", () => {
|
||||
const s = new ConfigState()
|
||||
s.handleConfigLoaded({ snapshot: true })
|
||||
s.updateConfig({ snapshot: false })
|
||||
s.saveConfig()
|
||||
expect(s.saving).toBe(true)
|
||||
|
||||
// Server confirms the write
|
||||
// An unrelated config push must not confirm the in-flight write.
|
||||
s.handleConfigUpdated({ snapshot: false })
|
||||
|
||||
expect(s.config.snapshot).toBe(false)
|
||||
expect(s.dirty).toBe(true)
|
||||
expect(s.saving).toBe(true)
|
||||
|
||||
s.handleConfigSaved()
|
||||
|
||||
expect(s.dirty).toBe(false)
|
||||
expect(s.saving).toBe(false)
|
||||
expect(Object.keys(s.draft).length).toBe(0)
|
||||
@@ -220,8 +225,9 @@ describe("ConfigState", () => {
|
||||
s.updateConfig({ default_agent: null })
|
||||
s.saveConfig()
|
||||
|
||||
// Server confirms the write by returning config without default_agent.
|
||||
// The refresh returns config without default_agent before the write ack.
|
||||
s.handleConfigUpdated({})
|
||||
s.handleConfigSaved()
|
||||
|
||||
expect(s.config.default_agent).toBeUndefined()
|
||||
expect(s.dirty).toBe(false)
|
||||
@@ -258,6 +264,20 @@ describe("ConfigState", () => {
|
||||
expect(s.saved.agent?.code?.prompt).toBeUndefined()
|
||||
expect(s.config.agent?.code?.prompt).toBeUndefined()
|
||||
})
|
||||
|
||||
it("rejects programmatic edits while the settings UI is inert", () => {
|
||||
const s = new ConfigState()
|
||||
s.handleConfigLoaded({ snapshot: true, username: "alice" })
|
||||
s.updateConfig({ snapshot: false })
|
||||
s.saveConfig()
|
||||
|
||||
s.updateConfig({ username: "bob" })
|
||||
s.handleConfigSaved()
|
||||
|
||||
expect(s.config).toEqual({ snapshot: false, username: "alice" })
|
||||
expect(s.saved).toEqual({ snapshot: false, username: "alice" })
|
||||
expect(s.dirty).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("configSaveFailed while a save is in-flight", () => {
|
||||
@@ -289,6 +309,7 @@ describe("ConfigState", () => {
|
||||
s.saveConfig()
|
||||
s.saveConfig()
|
||||
s.handleConfigUpdated({ snapshot: false })
|
||||
s.handleConfigSaved()
|
||||
|
||||
expect(s.saving).toBe(false)
|
||||
expect(s.dirty).toBe(false)
|
||||
@@ -359,8 +380,9 @@ describe("ConfigState", () => {
|
||||
s.updateConfig({ agent: { explore: { model: null } } })
|
||||
s.saveConfig()
|
||||
|
||||
// Backend removed the override and pushes the stripped config back.
|
||||
// Backend removed the override, then explicitly confirms the write.
|
||||
s.handleConfigUpdated({ agent: { explore: {} } })
|
||||
s.handleConfigSaved()
|
||||
|
||||
expect(s.config.agent?.explore?.model).toBeUndefined()
|
||||
expect(s.dirty).toBe(false)
|
||||
|
||||
@@ -8,6 +8,10 @@ type Internals = {
|
||||
connectionState: "connecting" | "connected" | "disconnected" | "error"
|
||||
currentSession: { id: string } | null
|
||||
cachedIndexingStatusMessage: unknown
|
||||
pending: number
|
||||
configRevision: number
|
||||
refreshWait: number
|
||||
postMessage: (message: unknown) => void
|
||||
handleEvent: (event: unknown, directory?: string) => void
|
||||
reloadAfterAuthChange: () => Promise<void>
|
||||
handleUpdateConfig: (
|
||||
@@ -16,6 +20,8 @@ type Internals = {
|
||||
globalUnset?: string[][],
|
||||
projectUnset?: string[][],
|
||||
) => Promise<void>
|
||||
fetchAndSendGlobalConfig: () => Promise<void>
|
||||
fetchAndSendConfigUpdated: () => Promise<void>
|
||||
fetchAndSendConfig: () => Promise<void>
|
||||
fetchAndSendProviders: () => Promise<void>
|
||||
fetchAndSendAgents: () => Promise<void>
|
||||
@@ -47,6 +53,7 @@ function createConnection() {
|
||||
}
|
||||
|
||||
return {
|
||||
client,
|
||||
drains: () => drains,
|
||||
patches: () => patches,
|
||||
service: {
|
||||
@@ -109,6 +116,128 @@ describe("KiloProvider indexing refresh", () => {
|
||||
expect(indexing).toBe(0)
|
||||
})
|
||||
|
||||
it("confirms saved config when the post-write refresh stalls", async () => {
|
||||
const conn = createConnection()
|
||||
conn.client.config.get = async () => new Promise<never>(() => {})
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
const internal = provider as unknown as Internals
|
||||
const messages: Array<{ type?: string; writes: number }> = []
|
||||
|
||||
internal.connectionState = "connected"
|
||||
internal.refreshWait = 0
|
||||
internal.postMessage = (message) =>
|
||||
messages.push({ ...(message as { type?: string }), writes: conn.patches().length })
|
||||
|
||||
await internal.handleUpdateConfig(
|
||||
{ indexing: { provider: "kilo" } },
|
||||
{ commit_message: { prompt: "Use conventional commits" } },
|
||||
)
|
||||
|
||||
expect(messages).toEqual([{ type: "configSaved", writes: 2 }])
|
||||
expect(internal.pending).toBe(0)
|
||||
})
|
||||
|
||||
it("does not confirm a partially written scoped save", async () => {
|
||||
const conn = createConnection()
|
||||
conn.client.config.overlayUpdate = async (patch: unknown) => {
|
||||
conn.patches().push(patch)
|
||||
if ((patch as { scope?: string }).scope === "project") throw new Error("project write failed")
|
||||
return { data: {} }
|
||||
}
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
const internal = provider as unknown as Internals
|
||||
const messages: Array<{ type?: string }> = []
|
||||
|
||||
internal.connectionState = "connected"
|
||||
internal.postMessage = (message) => messages.push(message as { type?: string })
|
||||
|
||||
await internal.handleUpdateConfig({ snapshot: true }, { commit_message: { prompt: "test" } })
|
||||
|
||||
expect(messages.map((message) => message.type)).toEqual(["configUpdated", "configUpdateFailed"])
|
||||
expect(internal.pending).toBe(0)
|
||||
})
|
||||
|
||||
it("reports a partial write failure when the recovery refresh stalls", async () => {
|
||||
const conn = createConnection()
|
||||
conn.client.config.overlayUpdate = async (patch: unknown) => {
|
||||
conn.patches().push(patch)
|
||||
if ((patch as { scope?: string }).scope === "project") throw new Error("project write failed")
|
||||
return { data: {} }
|
||||
}
|
||||
conn.client.config.get = async () => new Promise<never>(() => {})
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
const internal = provider as unknown as Internals
|
||||
const messages: Array<{ type?: string }> = []
|
||||
|
||||
internal.connectionState = "connected"
|
||||
internal.refreshWait = 0
|
||||
internal.postMessage = (message) => messages.push(message as { type?: string })
|
||||
|
||||
await internal.handleUpdateConfig({ snapshot: true }, { commit_message: { prompt: "test" } })
|
||||
|
||||
expect(messages.map((message) => message.type)).toEqual(["configUpdateFailed"])
|
||||
expect(internal.pending).toBe(0)
|
||||
})
|
||||
|
||||
it("drops a stale config refresh that finishes after a newer one", async () => {
|
||||
const conn = createConnection()
|
||||
let release!: () => void
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
release = resolve
|
||||
})
|
||||
let calls = 0
|
||||
conn.client.config.get = async () => {
|
||||
calls += 1
|
||||
if (calls === 1) {
|
||||
await gate
|
||||
return { data: { snapshot: true } }
|
||||
}
|
||||
return { data: { snapshot: false } }
|
||||
}
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
const internal = provider as unknown as Internals
|
||||
const messages: Array<{ type?: string; config?: Config }> = []
|
||||
|
||||
internal.connectionState = "connected"
|
||||
internal.postMessage = (message) => messages.push(message as { type?: string; config?: Config })
|
||||
|
||||
const stale = internal.fetchAndSendConfigUpdated()
|
||||
await Bun.sleep(0)
|
||||
await internal.fetchAndSendConfigUpdated()
|
||||
release()
|
||||
await stale
|
||||
|
||||
expect(messages).toHaveLength(1)
|
||||
expect(messages[0]?.config?.snapshot).toBe(false)
|
||||
expect(internal.configRevision).toBe(2)
|
||||
})
|
||||
|
||||
it("drops a stale global-only config response", async () => {
|
||||
const conn = createConnection()
|
||||
let release!: () => void
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
release = resolve
|
||||
})
|
||||
conn.client.global.config.get = async () => {
|
||||
await gate
|
||||
return { data: { snapshot: true } }
|
||||
}
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
const internal = provider as unknown as Internals
|
||||
const messages: Array<{ type?: string }> = []
|
||||
|
||||
internal.connectionState = "connected"
|
||||
internal.postMessage = (message) => messages.push(message as { type?: string })
|
||||
|
||||
const stale = internal.fetchAndSendGlobalConfig()
|
||||
await Bun.sleep(0)
|
||||
internal.configRevision++
|
||||
release()
|
||||
await stale
|
||||
|
||||
expect(messages).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("refreshes providers when prompt-training model visibility changes", async () => {
|
||||
const conn = createConnection()
|
||||
const provider = new KiloProvider({} as never, conn.service as never)
|
||||
|
||||
@@ -158,6 +158,8 @@ const Settings: Component<SettingsProps> = (props) => {
|
||||
variant="settings"
|
||||
value={active()}
|
||||
onChange={onTabChange}
|
||||
inert={saving()}
|
||||
aria-busy={saving()}
|
||||
style={{ flex: 1, overflow: "hidden" }}
|
||||
>
|
||||
<Tabs.List>
|
||||
|
||||
@@ -130,42 +130,34 @@ export const ConfigProvider: ParentComponent = (props) => {
|
||||
return
|
||||
}
|
||||
if (message.type === "configUpdated") {
|
||||
if (saving()) {
|
||||
// This configUpdated is the confirmation of our saveConfig() write.
|
||||
// Clear the draft now that the server has confirmed the write.
|
||||
setSaving(false)
|
||||
setDraft({})
|
||||
setGlobalDraft({})
|
||||
setProjectDraft({})
|
||||
setSaveError(null)
|
||||
setConfig(message.config)
|
||||
if (message.globalConfig !== undefined) {
|
||||
setGlobalConfig(mergeScopedConfig(message.globalConfig, globalDraft()))
|
||||
setSavedGlobal(message.globalConfig)
|
||||
}
|
||||
if (message.projectConfig !== undefined) {
|
||||
setProjectConfig(message.projectConfig)
|
||||
setSavedProject(message.projectConfig)
|
||||
}
|
||||
setFeatures(message.features)
|
||||
} else {
|
||||
// configUpdated from a different source (e.g. PermissionDock save).
|
||||
// Re-apply the draft on top so pending settings changes are preserved.
|
||||
setConfig(resolveConfig(message.config, draft(), has(draft() as Record<string, unknown>)))
|
||||
if (message.globalConfig !== undefined) {
|
||||
setGlobalConfig(mergeScopedConfig(message.globalConfig, globalDraft()))
|
||||
setSavedGlobal(message.globalConfig)
|
||||
}
|
||||
if (message.projectConfig !== undefined) {
|
||||
setProjectConfig(mergeScopedConfig(message.projectConfig, projectDraft()))
|
||||
setSavedProject(message.projectConfig)
|
||||
}
|
||||
setFeatures(message.features)
|
||||
// Config pushes can come from unrelated saves or disposal events. Re-apply
|
||||
// local drafts and wait for configSaved before acknowledging this write.
|
||||
setConfig(resolveConfig(message.config, draft(), has(draft() as Record<string, unknown>)))
|
||||
if (message.globalConfig !== undefined) {
|
||||
setGlobalConfig(mergeScopedConfig(message.globalConfig, globalDraft()))
|
||||
setSavedGlobal(message.globalConfig)
|
||||
}
|
||||
if (message.projectConfig !== undefined) {
|
||||
setProjectConfig(mergeScopedConfig(message.projectConfig, projectDraft()))
|
||||
setSavedProject(message.projectConfig)
|
||||
}
|
||||
setFeatures(message.features)
|
||||
if (message.settings) mergeSettings(message.settings)
|
||||
setSaved(message.config)
|
||||
return
|
||||
}
|
||||
if (message.type === "configSaved") {
|
||||
if (!saving()) return
|
||||
setSaving(false)
|
||||
setDraft({})
|
||||
setGlobalDraft({})
|
||||
setProjectDraft({})
|
||||
setSaveError(null)
|
||||
setSaved(config())
|
||||
setSavedGlobal(globalConfig())
|
||||
setSavedProject(projectConfig())
|
||||
return
|
||||
}
|
||||
if (message.type === "configUpdateFailed") {
|
||||
// The write was rejected (e.g. schema validation) — surface the error
|
||||
// and keep the draft + isDirty so the user can correct and retry.
|
||||
@@ -213,6 +205,7 @@ export const ConfigProvider: ParentComponent = (props) => {
|
||||
})
|
||||
|
||||
function updateConfig(partial: Partial<Config>) {
|
||||
if (saving()) return
|
||||
// Optimistically update local state with deep merge + null stripping
|
||||
setConfig((prev) => stripNulls(deepMerge(prev, partial)))
|
||||
// Accumulate in draft — will be sent on saveConfig()
|
||||
@@ -223,24 +216,28 @@ export const ConfigProvider: ParentComponent = (props) => {
|
||||
}
|
||||
|
||||
function updateGlobalConfig(partial: Partial<Config>) {
|
||||
if (saving()) return
|
||||
setGlobalConfig((prev) => mergeScopedConfig(prev, partial))
|
||||
setGlobalDraft((prev) => deepMerge(prev as Config, partial))
|
||||
setSaveError(null)
|
||||
}
|
||||
|
||||
function updateProjectConfig(partial: Partial<Config>) {
|
||||
if (saving()) return
|
||||
setProjectConfig((prev) => mergeScopedConfig(prev, partial))
|
||||
setProjectDraft((prev) => deepMerge(prev as Config, partial))
|
||||
setSaveError(null)
|
||||
}
|
||||
|
||||
function updateSetting(key: string, value: unknown) {
|
||||
if (saving()) return
|
||||
setSettings((prev) => ({ ...prev, [key]: value }))
|
||||
setSettingsDraft((prev) => ({ ...prev, [key]: value }))
|
||||
setSaveError(null)
|
||||
}
|
||||
|
||||
function saveConfig() {
|
||||
if (saving()) return
|
||||
const changes = draft()
|
||||
const globals = globalDraft()
|
||||
const projects = projectDraft()
|
||||
@@ -250,7 +247,7 @@ export const ConfigProvider: ParentComponent = (props) => {
|
||||
const projectDirty = has(projects as Record<string, unknown>)
|
||||
const settingsDirty = has(pending)
|
||||
if (!configDirty && !globalDirty && !projectDirty && !settingsDirty) return
|
||||
// Don't clear draft/isDirty yet — wait for configUpdated confirmation.
|
||||
// Don't clear draft/isDirty yet — wait for configSaved confirmation.
|
||||
// If the write fails, the save bar stays visible so the user can retry.
|
||||
setSaving(true)
|
||||
setSaveError(null)
|
||||
|
||||
@@ -539,6 +539,10 @@ export interface ConfigUpdatedMessage {
|
||||
features: FeatureFlags
|
||||
}
|
||||
|
||||
export interface ConfigSavedMessage {
|
||||
type: "configSaved"
|
||||
}
|
||||
|
||||
export interface ConfigUpdateFailedMessage {
|
||||
type: "configUpdateFailed"
|
||||
message: string
|
||||
@@ -1133,6 +1137,7 @@ export type ExtensionMessage =
|
||||
| ClaudeCompatSettingLoadedMessage
|
||||
| ConfigLoadedMessage
|
||||
| ConfigUpdatedMessage
|
||||
| ConfigSavedMessage
|
||||
| ConfigUpdateFailedMessage
|
||||
| GlobalConfigLoadedMessage
|
||||
| NotificationSettingsLoadedMessage
|
||||
|
||||
@@ -100,6 +100,7 @@ export class ConfigState {
|
||||
|
||||
/** Accumulate a partial change (same as the toggle click path). */
|
||||
updateConfig(partial: Partial<Config>) {
|
||||
if (this.saving) return
|
||||
this.config = stripNulls(deepMerge(this.config, partial))
|
||||
this.draft = deepMerge(this.draft as Config, partial)
|
||||
this.dirty = true
|
||||
@@ -115,14 +116,7 @@ export class ConfigState {
|
||||
|
||||
/** Handle an incoming configUpdated push from the extension. */
|
||||
handleConfigUpdated(server: Config) {
|
||||
if (this.saving) {
|
||||
this.saving = false
|
||||
this.draft = {}
|
||||
this.dirty = false
|
||||
this.config = server
|
||||
} else {
|
||||
this.config = resolveConfig(server, this.draft, this.dirty)
|
||||
}
|
||||
this.config = resolveConfig(server, this.draft, this.dirty)
|
||||
this.saved = server
|
||||
}
|
||||
|
||||
|
||||
@@ -177,7 +177,7 @@ export const layer: Layer.Layer<Service, never, Project.Service | InstanceBootst
|
||||
}
|
||||
yield* disposeEntry(item[0], item[1], exit.value)
|
||||
}),
|
||||
{ discard: true },
|
||||
{ discard: true, concurrency: 4 }, // kilocode_change - dispose independent worktrees concurrently
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
import { describe, expect } from "bun:test"
|
||||
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
|
||||
import { Deferred, Effect, Fiber, Layer } from "effect"
|
||||
import { registerDisposer } from "../../../src/effect/instance-registry"
|
||||
import { InstanceBootstrap } from "../../../src/project/bootstrap-service"
|
||||
import { InstanceStore } from "../../../src/project/instance-store"
|
||||
import { tmpdirScoped } from "../../fixture/fixture"
|
||||
import { awaitWithTimeout, testEffect } from "../../lib/effect"
|
||||
|
||||
const bootstrap = Layer.succeed(InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.void }))
|
||||
const it = testEffect(
|
||||
Layer.mergeAll(InstanceStore.defaultLayer, CrossSpawnSpawner.defaultLayer).pipe(Layer.provide(bootstrap)),
|
||||
)
|
||||
|
||||
const register = (disposer: (directory: string) => Promise<void>) =>
|
||||
Effect.acquireRelease(
|
||||
Effect.sync(() => registerDisposer(disposer)),
|
||||
(off) => Effect.sync(off),
|
||||
)
|
||||
|
||||
describe("InstanceStore disposal", () => {
|
||||
it.live("disposes four directories concurrently", () =>
|
||||
Effect.gen(function* () {
|
||||
const dirs = yield* Effect.all(
|
||||
Array.from({ length: 4 }, () => tmpdirScoped({ git: true })),
|
||||
{ concurrency: "unbounded" },
|
||||
)
|
||||
const store = yield* InstanceStore.Service
|
||||
const ready = yield* Deferred.make<void>()
|
||||
const release = yield* Deferred.make<void>()
|
||||
const started = new Set<string>()
|
||||
|
||||
yield* Effect.addFinalizer(() => Deferred.succeed(release, undefined).pipe(Effect.ignore))
|
||||
yield* register(async (directory) => {
|
||||
if (!dirs.includes(directory)) return
|
||||
started.add(directory)
|
||||
if (started.size === dirs.length) Deferred.doneUnsafe(ready, Effect.void)
|
||||
await Effect.runPromise(Deferred.await(release))
|
||||
})
|
||||
|
||||
yield* Effect.forEach(dirs, (directory) => store.load({ directory }), { discard: true })
|
||||
const fiber = yield* store.disposeAll().pipe(Effect.forkScoped)
|
||||
|
||||
yield* awaitWithTimeout(Deferred.await(ready), "instance disposal remained serial", "1 second")
|
||||
expect(started).toEqual(new Set(dirs))
|
||||
|
||||
yield* Deferred.succeed(release, undefined)
|
||||
yield* Fiber.join(fiber)
|
||||
}),
|
||||
)
|
||||
})
|
||||
Reference in New Issue
Block a user