diff --git a/.changeset/skip-disabled-snapshot-locks.md b/.changeset/skip-disabled-snapshot-locks.md new file mode 100644 index 000000000000..5db63fe4bf3d --- /dev/null +++ b/.changeset/skip-disabled-snapshot-locks.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Prevent interrupted snapshot progress from poisoning future prompts, and skip snapshot lock waits when snapshots are disabled. diff --git a/packages/opencode/src/kilocode/session/part-lifecycle.ts b/packages/opencode/src/kilocode/session/part-lifecycle.ts index c518b6f7ab40..2c3d2273c444 100644 --- a/packages/opencode/src/kilocode/session/part-lifecycle.ts +++ b/packages/opencode/src/kilocode/session/part-lifecycle.ts @@ -1,4 +1,4 @@ -import { MessageV2 } from "@/session/message-v2" +import type { MessageV2 } from "@/session/message-v2" export namespace KiloPartLifecycle { export const key = "kilocode.lifecycle" diff --git a/packages/opencode/src/kilocode/snapshot/track.ts b/packages/opencode/src/kilocode/snapshot/track.ts index 5fcb2a62c6b2..9028588e9ca8 100644 --- a/packages/opencode/src/kilocode/snapshot/track.ts +++ b/packages/opencode/src/kilocode/snapshot/track.ts @@ -285,6 +285,9 @@ export namespace KiloSnapshotTrack { const timeoutMs = input.timeoutMs ?? TIMEOUT_MS const progressDelayMs = input.progressDelayMs ?? PROGRESS_DELAY_MS const cleanupTimeoutMs = input.progressCleanupTimeoutMs ?? PROGRESS_CLEANUP_TIMEOUT_MS + // Progress cleanup can outlive this fiber, but its events must retain the project directory. + const bridge = yield* EffectBridge.make() + const call = (fn: () => Promise) => bridge.promise(Effect.promise(fn)) // The progress part is only published when we have both a session and // a target message. Background/non-turn callers skip the indicator. @@ -321,8 +324,7 @@ export namespace KiloSnapshotTrack { timeout.resolve(false) }, cleanupTimeoutMs) const removed = await Promise.race([ - hooks - .endProgress({ handle }, ctl.signal) + call(() => hooks.endProgress({ handle }, ctl.signal)) .then(() => true as const) .catch((err) => { log.warn("failed to clear snapshot progress part", { err }) @@ -373,7 +375,7 @@ export namespace KiloSnapshotTrack { handle.started = true const started = yield* Effect.promise((signal) => settleProgress( - () => hooks.startProgress({ handle, text: nextFrameText() }, signal), + () => call(() => hooks.startProgress({ handle, text: nextFrameText() }, signal)), "failed to publish snapshot progress part", ), ) @@ -384,7 +386,7 @@ export namespace KiloSnapshotTrack { const text = nextFrameText() yield* Effect.promise((signal) => settleProgress( - () => hooks.updateProgress({ handle, text }, signal), + () => call(() => hooks.updateProgress({ handle, text }, signal)), "failed to advance snapshot spinner frame", ), ) diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 502ffd534158..b008d40bc145 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -39,6 +39,7 @@ import { Snapshot } from "@/snapshot" // kilocode_change import { SessionNetwork } from "./network" // kilocode_change import { CodexAuthExpiredError } from "@/kilocode/provider/codex-refresh" // kilocode_change import { KiloSessionMessageOrder } from "@/kilocode/session/message-order" // kilocode_change +import { KiloPartLifecycle } from "@/kilocode/session/part-lifecycle" // kilocode_change import * as TextStream from "@/kilocode/text-stream" // kilocode_change import { BoardNotice } from "@/kilocode/board/notice" // kilocode_change import { Effect, Schema } from "effect" @@ -399,6 +400,7 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( return part.metadata?.anthropic?.signature != null }) for (const part of msg.parts) { + if (KiloPartLifecycle.transient(part)) continue // kilocode_change - never replay transient UI parts // kilocode_change - !part.ignored keeps local UI warnings out of future prompts if (part.type === "text" && !part.ignored) { const text = part.text === "" && hasSignedReasoning ? " " : part.text diff --git a/packages/opencode/src/snapshot/index.ts b/packages/opencode/src/snapshot/index.ts index 39779464ca95..3336c0836fb7 100644 --- a/packages/opencode/src/snapshot/index.ts +++ b/packages/opencode/src/snapshot/index.ts @@ -334,6 +334,7 @@ export const layer: Layer.Layer = // kilocode_change end const cleanup = Effect.fnUntraced(function* () { + if ((yield* config.get()).snapshot === false) return undefined // kilocode_change - skip locks for disabled periodic cleanup too return yield* locked( Effect.gen(function* () { if (!(yield* enabled())) return @@ -930,6 +931,8 @@ export const layer: Layer.Layer = }), // kilocode_change start - isolate turn-facing snapshot work from poisoned locks track: Effect.fn("Snapshot.track")(function* (opts) { + // Check before starting progress or waiting on an earlier snapshot's lock. + if ((yield* config.get()).snapshot === false) return undefined const ctx = yield* InstanceState.context const guard = trackState(ctx.worktree) return yield* KiloSnapshotTrack.protect({ @@ -946,6 +949,7 @@ export const layer: Layer.Layer = }) }), patch: Effect.fn("Snapshot.patch")(function* (hash: string) { + if ((yield* config.get()).snapshot === false) return { hash, files: [] } const ctx = yield* InstanceState.context const guard = trackState(ctx.worktree) return yield* KiloSnapshotTrack.protect({ diff --git a/packages/opencode/test/kilocode/message-v2-transient.test.ts b/packages/opencode/test/kilocode/message-v2-transient.test.ts new file mode 100644 index 000000000000..757407efd053 --- /dev/null +++ b/packages/opencode/test/kilocode/message-v2-transient.test.ts @@ -0,0 +1,84 @@ +import { expect, test } from "bun:test" +import { ModelV2 } from "@opencode-ai/core/model" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { SessionV1 } from "@opencode-ai/core/v1/session" +import { MessageV2 } from "../../src/session/message-v2" +import { KiloPartLifecycle } from "../../src/kilocode/session/part-lifecycle" +import { MessageID, PartID, SessionID } from "../../src/session/schema" +import type { Provider } from "../../src/provider/provider" + +const sessionID = SessionID.make("ses_transient") +const providerID = ProviderV2.ID.make("test") +const model: Provider.Model = { + id: ModelV2.ID.make("test-model"), + providerID, + api: { id: "test-model", url: "https://example.com", npm: "@ai-sdk/openai" }, +} as Provider.Model + +const base = (messageID: string, id: string) => ({ + id: PartID.make(id), + sessionID, + messageID: MessageID.make(messageID), +}) + +test("does not replay persisted transient snapshot progress", async () => { + const userID = MessageID.make("msg_user") + const assistantID = MessageID.make("msg_assistant") + const input: SessionV1.WithParts[] = [ + { + info: { + id: userID, + sessionID, + role: "user", + time: { created: 0 }, + agent: "user", + model: { providerID, modelID: model.id }, + tools: {}, + mode: "", + } as SessionV1.User, + parts: [{ ...base(userID, "prt_user"), type: "text", text: "continue" }], + }, + { + info: { + id: assistantID, + sessionID, + role: "assistant", + parentID: userID, + time: { created: 0 }, + modelID: model.id, + providerID, + mode: "", + agent: "build", + path: { cwd: "/", root: "/" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + } as SessionV1.Assistant, + parts: [ + { + ...base(assistantID, "prt_tool"), + type: "tool", + tool: "bash", + callID: "call_bash", + state: { + status: "completed", + input: {}, + output: "changed files", + title: "bash", + time: { start: 0, end: 1 }, + }, + }, + { + ...base(assistantID, "prt_progress"), + type: "text", + text: "Initializing snapshot…", + synthetic: true, + metadata: { [KiloPartLifecycle.key]: "transient" }, + }, + ] as SessionV1.Part[], + }, + ] + + const messages = await MessageV2.toModelMessages(input, model) + expect(JSON.stringify(messages)).not.toContain("Initializing snapshot") + expect(messages.some((message) => message.role === "tool")).toBe(true) +}) diff --git a/packages/opencode/test/kilocode/snapshot-disabled.test.ts b/packages/opencode/test/kilocode/snapshot-disabled.test.ts new file mode 100644 index 000000000000..18b9c3e98809 --- /dev/null +++ b/packages/opencode/test/kilocode/snapshot-disabled.test.ts @@ -0,0 +1,37 @@ +import { expect } from "bun:test" +import path from "path" +import { Effect, Layer } from "effect" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { Global } from "@opencode-ai/core/global" +import { Hash } from "@opencode-ai/core/util/hash" +import { EffectFlock } from "@opencode-ai/core/util/effect-flock" +import { Snapshot } from "../../src/snapshot" +import { requireInstance } from "../fixture/fixture" +import { awaitWithTimeout, testEffect } from "../lib/effect" + +const it = testEffect(Layer.mergeAll(Snapshot.defaultLayer, AppNodeBuilder.build(EffectFlock.node))) + +for (const operation of ["track", "patch", "cleanup"] as const) { + it.instance( + `disabled snapshots bypass a held repository lock during ${operation}`, + () => + Effect.gen(function* () { + const ctx = yield* requireInstance + const snapshot = yield* Snapshot.Service + const flock = yield* EffectFlock.Service + const gitdir = path.join(Global.Path.data, "snapshot", ctx.project.id, Hash.fast(ctx.worktree)) + yield* flock.acquire(`snapshot:${gitdir}`) + + const effect: Effect.Effect = + operation === "patch" ? snapshot.patch("existing-snapshot") : snapshot[operation]() + const result = yield* awaitWithTimeout( + effect, + `${operation} waited for a snapshot lock while disabled`, + "1 second", + ) + + expect(result).toEqual(operation === "patch" ? { hash: "existing-snapshot", files: [] } : undefined) + }), + { git: true, config: { snapshot: false } }, + ) +} diff --git a/packages/opencode/test/kilocode/snapshot-track-timeout.test.ts b/packages/opencode/test/kilocode/snapshot-track-timeout.test.ts index e2a410a10145..852ad14ebe0c 100644 --- a/packages/opencode/test/kilocode/snapshot-track-timeout.test.ts +++ b/packages/opencode/test/kilocode/snapshot-track-timeout.test.ts @@ -5,13 +5,19 @@ // touch the real Question module or write to the filesystem. import { describe, expect, test } from "bun:test" +import { ModelV2 } from "@opencode-ai/core/model" +import { ProviderV2 } from "@opencode-ai/core/provider" import { Deferred, Duration, Effect, Fiber } from "effect" import * as TestClock from "effect/testing/TestClock" import path from "path" import { PartID, type MessageID, type SessionID } from "../../src/session/schema" import { KiloSnapshotTrack } from "../../src/kilocode/snapshot/track" import { KiloPartLifecycle } from "../../src/kilocode/session/part-lifecycle" -import { TestInstance } from "../fixture/fixture" +import { AppRuntime } from "../../src/effect/app-runtime" +import { GlobalBus, type GlobalEvent } from "../../src/bus/global" +import { InstanceRef } from "../../src/effect/instance-ref" +import { Session } from "../../src/session/session" +import { requireInstance, TestInstance } from "../fixture/fixture" import { awaitWithTimeout, it } from "../lib/effect" const SESSION = "ses_test" as SessionID @@ -951,6 +957,75 @@ describe("KiloSnapshotTrack progress indicator", () => { }) }) +describe("KiloSnapshotTrack default hooks", () => { + it.instance( + "preserves the instance directory for real session progress events", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const ctx = yield* requireInstance + const session = yield* Effect.promise(() => + AppRuntime.runPromise( + Session.Service.use((svc) => svc.create({ title: "snapshot progress" })).pipe( + Effect.provideService(InstanceRef, ctx), + ), + ), + ) + const message = yield* Effect.promise(() => + AppRuntime.runPromise( + Session.Service.use((svc) => + svc.updateMessage({ + id: MESSAGE, + role: "user", + sessionID: session.id, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("test") }, + time: { created: Date.now() }, + }), + ).pipe(Effect.provideService(InstanceRef, ctx)), + ), + ) + + const seen: GlobalEvent[] = [] + const removed = yield* Deferred.make() + const on = (event: GlobalEvent) => { + const properties = event.payload?.properties + if (properties?.sessionID !== session.id && properties?.part?.sessionID !== session.id) return + seen.push(event) + if (event.payload?.type === "message.part.removed") Deferred.doneUnsafe(removed, Effect.succeed(undefined)) + } + GlobalBus.on("event", on) + yield* Effect.addFinalizer(() => + Effect.promise(async () => { + GlobalBus.off("event", on) + await AppRuntime.runPromise( + Session.Service.use((svc) => svc.remove(session.id)).pipe(Effect.provideService(InstanceRef, ctx)), + ) + }), + ) + + const result = yield* KiloSnapshotTrack.wrap({ + inner: slowInner(350, "progress-hash"), + state: KiloSnapshotTrack.makeState(), + sessionID: session.id, + messageID: message.id, + timeoutMs: 1_000, + progressDelayMs: 1, + }) + + expect(result).toBe("progress-hash") + yield* awaitWithTimeout(Deferred.await(removed), "timed out waiting for snapshot progress removal") + const progress = seen.filter( + (event) => event.payload?.type === "message.part.updated" || event.payload?.type === "message.part.removed", + ) + expect(progress.some((event) => event.payload?.type === "message.part.updated")).toBe(true) + expect(progress.some((event) => event.payload?.type === "message.part.removed")).toBe(true) + for (const event of progress) expect(event.directory).toBe(test.directory) + }), + { git: true }, + ) +}) + describe("KiloSnapshotTrack persistDisable", () => { it.instance( "disable writes snapshot:false to the project config", diff --git a/script/check-opencode-promise-facades.ts b/script/check-opencode-promise-facades.ts index d0b6a801ef2e..92d1d3f2458a 100644 --- a/script/check-opencode-promise-facades.ts +++ b/script/check-opencode-promise-facades.ts @@ -42,6 +42,10 @@ const testAllow: Record = { count: 2, reason: "disk-backed instance integration test cleanup", }, + "kilocode/snapshot-track-timeout.test.ts": { + count: 4, + reason: "production default snapshot hooks require the shared runtime and instance context", + }, "kilocode/kilo-sessions.test.ts": { count: 36, reason: