From 53a2ad39fb98118ddc91f79c286d06af97395daa Mon Sep 17 00:00:00 2001 From: Lex Date: Sun, 16 Aug 2026 02:03:48 +0800 Subject: [PATCH 1/2] fix(goal): tolerate vanished-session message windows, surface loop failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the invisible goal stall: the afterIdle evaluation died on a typed NotFoundError from the messages-window read (session row gone mid-goal, or a synthetic session), and the fork's bare Effect.ignore swallowed it — goal left permanently active, turns_used frozen at 0, zero logs. - afterIdle + zombie probe + post-judge reload: NotFoundError on any of the three messages windows degrades to an empty window (MessageV2.stream pattern) so the existing visible branches handle it (pause / continuation) - triggerEvaluation fork: Effect.ignore -> catchCause + logWarning (interrupts stay silent per F1) so future evaluation failures are diagnosable - judge chain: orElseSucceed -> catchCause so DEFECTS in the production provider chain (config orDie, payload decode) fold into the parseFailed budget instead of silently killing the evaluation - auto-pause transcript line: ignore -> catchCause + logWarning (symmetric with the done branch) - P2-B comment corrected: effect v4 Effect.ignore absorbs defects too; catchCause's value here is diagnosability, not subscription survival - regression tests: production-wiring probe (bootstrap-wiring.test.ts), pre-judge window pause, post-judge reload tolerance (GOAL-FP-01-18), judge-defect budget degradation (GOAL-FP-01-18b) - lint: new code is warning-neutral; drop 11 pre-existing unused imports to restore gate margin (oxlint ratchet, local 4846 -> 4829) --- packages/opencode/src/goal/judge.ts | 18 +- packages/opencode/src/goal/loop.ts | 72 +++++-- .../opencode/test/cli/github-action.test.ts | 1 - .../test/dag/dag-loop-integration.test.ts | 1 - .../test/goal/bootstrap-wiring.test.ts | 70 ++++++ packages/opencode/test/goal/e2e-loop.test.ts | 203 ++++++++++++++++++ packages/opencode/test/lib/effect.ts | 1 - .../test/server/httpapi-event.test.ts | 2 +- .../test/server/session-messages.test.ts | 1 - .../structured-output-integration.test.ts | 1 - .../test/session/structured-output.test.ts | 1 - packages/opencode/test/tool/grep.test.ts | 2 +- packages/opencode/test/tool/read.test.ts | 1 - packages/opencode/test/tool/skill.test.ts | 1 - .../opencode/test/tool/truncation.test.ts | 1 - 15 files changed, 348 insertions(+), 28 deletions(-) create mode 100644 packages/opencode/test/goal/bootstrap-wiring.test.ts diff --git a/packages/opencode/src/goal/judge.ts b/packages/opencode/src/goal/judge.ts index 16846e986d..45228ec1f6 100644 --- a/packages/opencode/src/goal/judge.ts +++ b/packages/opencode/src/goal/judge.ts @@ -75,10 +75,18 @@ export const run = Effect.fn("Goal.Judge.run")(function* ( // "judge is unreliable" uniformly regardless of failure mode. The verdict // stays "continue" so a single transient blip does not stall the loop; // it only pauses after MAX_CONSECUTIVE_PARSE_FAILURES in a row. - Effect.orElseSucceed((): JudgeResult => ({ - verdict: "continue", - reason: "judge transport error (timeout or network) — counting toward pause budget", - parseFailed: true, - })), + // + // catchCause (not orElseSucceed): the production callLLM chain can + // DEFECT — config first-use orDie, payload decode throws — and a defect + // escaping here kills afterIdle invisibly (the loop stalls at 0 turns + // with zero logs and no pause budget). catchCause folds defects into + // the same parseFailed budget. + Effect.catchCause(() => + Effect.succeed({ + verdict: "continue", + reason: "judge transport error (timeout or network) — counting toward pause budget", + parseFailed: true, + } satisfies JudgeResult), + ), ) }) diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index 8349856ff4..18ff08f635 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -1,6 +1,7 @@ export * as GoalLoop from "./loop" import { Effect, Layer, Context, Option, Stream, Scope, Fiber, Cause } from "effect" +import { SessionV1 } from "@opencode-ai/core/v1/session" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { InstanceState } from "@/effect/instance-state" import { EventV2Bridge } from "@/event-v2-bridge" @@ -14,6 +15,7 @@ import { GoalPrompts } from "./prompts" import { generateText } from "ai" import { SessionID } from "@/session/schema" import { SessionAutomationLease } from "@/session/automation-lease" +import { NotFoundError } from "@/storage/storage" export interface Interface { readonly init: () => Effect.Effect @@ -148,12 +150,12 @@ const serviceLayer = Layer.effect( yield* triggerEvaluation(sid) // P2-B subscription survival: this handler now contains the // first defect-capable durable reads in the goal idle path - // (Goal.ownsSession / goal.load both orDie). Effect.ignore does - // NOT absorb defects — a transient store failure would - // permanently kill the runForEach subscription and the loop - // would never evaluate another idle event. catchCause absorbs - // failures AND defects at the boundary, so a store defect - // degrades to a logged, skipped evaluation — never a dead loop. + // (Goal.ownsSession / goal.load both orDie). In effect v4, + // Effect.ignore absorbs failures, defects AND interruptions — + // an error here would vanish without a trace, leaving skipped + // evaluations permanently invisible. catchCause keeps the same + // absorption but LOGS at the boundary, so a store defect + // degrades to a logged, skipped evaluation — never a silent one. }).pipe( Effect.catchCause((cause) => Effect.logWarning("GoalLoop idle handler failed", { sessionID: evt.data.sessionID, cause }), @@ -263,7 +265,13 @@ const serviceLayer = Layer.effect( goalState.turns_used === 0 && Date.now() - goalState.created_at > GoalPrompts.FRESHNESS_THRESHOLD ) { - const probeMsgs = yield* sessions.messages({ sessionID, limit: 1 }) + const probeMsgs = yield* sessions + .messages({ sessionID, limit: 1 }) + .pipe( + Effect.catchIf((e) => NotFoundError.isInstance(e), () => + Effect.succeed([] as SessionV1.WithParts[]), + ), + ) const hasAssistant = probeMsgs.some((m) => m.info.role === "assistant") if (isStaleZombie(goalState, hasAssistant)) { yield* pauseGoal( @@ -274,7 +282,18 @@ const serviceLayer = Layer.effect( } } - const msgs = yield* sessions.messages({ sessionID, limit: 20 }) + // A session whose row is gone (deleted mid-goal, or a synthetic + // session) fails page() with NotFoundError. Treat it as an empty window + // (same pattern as MessageV2.stream) so the no-lastAssistant branch + // below pauses visibly instead of this typed failure escaping and + // leaving the goal permanently "active". + const msgs = yield* sessions + .messages({ sessionID, limit: 20 }) + .pipe( + Effect.catchIf((e) => NotFoundError.isInstance(e), () => + Effect.succeed([] as SessionV1.WithParts[]), + ), + ) const lastAssistant = [...msgs].reverse().find((m) => m.info.role === "assistant") if (!lastAssistant) { // No assistant message in the last 20 — the conversation may have @@ -391,7 +410,14 @@ const serviceLayer = Layer.effect( sessionID, noReply: true, parts: [{ type: "text", text: updateResult.message }], - }).pipe(Effect.ignore) + }).pipe( + Effect.catchCause((cause) => + Effect.logWarning("goal pause message delivery failed", { + sessionID, + cause: Cause.pretty(cause), + }), + ), + ) } return } @@ -409,8 +435,17 @@ const serviceLayer = Layer.effect( } // Reload messages after judge LLM call — the snapshot from before judge - // may be stale if user sent messages during the 5-30s judge latency - const freshMsgs = yield* sessions.messages({ sessionID, limit: 20 }) + // may be stale if user sent messages during the 5-30s judge latency. + // Same vanished-session tolerance as the pre-judge window: NotFoundError + // becomes an empty window (shouldPreempt is defensively false for it), + // never a typed failure escaping the fork. + const freshMsgs = yield* sessions + .messages({ sessionID, limit: 20 }) + .pipe( + Effect.catchIf((e) => NotFoundError.isInstance(e), () => + Effect.succeed([] as SessionV1.WithParts[]), + ), + ) if (shouldPreempt(freshMsgs)) { // Same self-interrupt hazard as the done branch above: we ARE the @@ -561,7 +596,20 @@ const serviceLayer = Layer.effect( // afterIdle re-checks at its own entry load (see there) to close the // window between this load and the fork. if (scanResume && evaluatedRevisions.get(sessionID) === (goalState.revision ?? 0)) return - const fiber = yield* afterIdle(sessionID, scanResume).pipe(Effect.ignore, Effect.forkIn(scope)) + // GOAL-FP-01-17: never Effect.ignore here. A typed failure escaping + // afterIdle (e.g. a messages read against a vanished session row) used + // to vanish into ignore and left the goal permanently "active" with + // zero logs — an invisible stall. Interrupts (fiber replacement by a + // newer idle, scope disposal) stay silent: they are the normal + // overwrite path, same F1 discipline as the continuation catch below. + const fiber = yield* afterIdle(sessionID, scanResume).pipe( + Effect.catchCause((cause) => + Cause.hasInterrupts(cause) + ? Effect.void + : Effect.logWarning("goal afterIdle failed", { sessionID, cause: Cause.pretty(cause) }), + ), + Effect.forkIn(scope), + ) yield* goal.registerLoopFiber(sessionID, fiber) yield* Fiber.await(fiber).pipe( Effect.flatMap(() => goal.clearLoopFiberIf(sessionID, fiber)), diff --git a/packages/opencode/test/cli/github-action.test.ts b/packages/opencode/test/cli/github-action.test.ts index 57567d8c9b..564ee697c3 100644 --- a/packages/opencode/test/cli/github-action.test.ts +++ b/packages/opencode/test/cli/github-action.test.ts @@ -1,7 +1,6 @@ import { test, expect, describe } from "bun:test" import { SessionV1 } from "@opencode-ai/core/v1/session" import { extractResponseText, formatPromptTooLargeError } from "../../src/cli/cmd/github" -import type { MessageV2 } from "../../src/session/message-v2" import { SessionID, MessageID, PartID } from "../../src/session/schema" // Helper to create minimal valid parts diff --git a/packages/opencode/test/dag/dag-loop-integration.test.ts b/packages/opencode/test/dag/dag-loop-integration.test.ts index 4b22645dbd..022e10c9f5 100644 --- a/packages/opencode/test/dag/dag-loop-integration.test.ts +++ b/packages/opencode/test/dag/dag-loop-integration.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "bun:test" import { - buildGraph, type SchedulingNode, WorkflowRuntime, } from "@opencode-ai/core/dag/core/scheduling" diff --git a/packages/opencode/test/goal/bootstrap-wiring.test.ts b/packages/opencode/test/goal/bootstrap-wiring.test.ts new file mode 100644 index 0000000000..99d1c95346 --- /dev/null +++ b/packages/opencode/test/goal/bootstrap-wiring.test.ts @@ -0,0 +1,70 @@ +import { describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { NodeFileSystem } from "@effect/platform-node" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Goal } from "@/goal/goal" +import { SessionStatus } from "@/session/status" +import { SessionID } from "@/session/schema" +import { InstanceStore } from "@/project/instance-store" +import { provideTmpdirInstance } from "../fixture/fixture" +import { pollWithTimeout, testEffect } from "../lib/effect" + +const it = testEffect(Layer.mergeAll(CrossSpawnSpawner.defaultLayer, NodeFileSystem.layer)) + +// GOAL-BOOT-WIRING probe: boots the instance through the PRODUCTION path +// (AppRuntime → InstanceStore.provide → InstanceBootstrap.run, which is the +// only place that serviceOption-resolves and inits GoalLoop). Every other +// test/goal suite builds GoalLoop.layer directly and therefore never +// exercises this wiring. Pipeline under test, no judge involved: +// boot instance → set goal → publish session idle → +// afterIdle must reach the no-lastAssistant branch and PAUSE the goal. +// If the serviceOption wiring, subscription, ownership gate, lease claim, +// or event delivery is broken, the goal stays "active" and this test goes red. +describe("GoalLoop production wiring — idle must drive afterIdle", () => { + it.live( + "an idle session with an active goal leaves the active state", + () => + provideTmpdirInstance((path) => + Effect.promise(async () => { + const { AppRuntime } = await import("@/effect/app-runtime") + await AppRuntime.runPromise( + Effect.gen(function* () { + const store = yield* InstanceStore.Service + yield* store.provide( + { directory: path }, + Effect.gen(function* () { + // Instance booted via production bootstrap — GoalLoop.init + // must already have run here; do NOT call it again. + const goal = yield* Goal.Service + const status = yield* SessionStatus.Service + + const sid = SessionID.descending() + yield* goal.set(sid, "wiring probe", 5) + + // Session starts busy; flip to idle — this is the exact event + // the production Runner emits after a turn ends. + yield* status.set(sid, { type: "busy" }) + yield* status.set(sid, { type: "idle" }) + + // No assistant message exists → the healthy pipeline pauses + // the goal with the "近期消息中无 assistant 回复" reason. A + // stalled pipeline leaves it active. + const final = yield* pollWithTimeout( + Effect.gen(function* () { + const state = yield* goal.load(sid) + if (state && state.status !== "active") return state + return undefined + }), + "goal never left active after idle — GoalLoop pipeline not armed on the production wiring", + "8 seconds", + ) + expect(final.status).toBe("paused") + }), + ) + }), + ) + }), + ), + 20_000, + ) +}) diff --git a/packages/opencode/test/goal/e2e-loop.test.ts b/packages/opencode/test/goal/e2e-loop.test.ts index 144b1c70ed..f9cc5cfc65 100644 --- a/packages/opencode/test/goal/e2e-loop.test.ts +++ b/packages/opencode/test/goal/e2e-loop.test.ts @@ -2,6 +2,7 @@ import { describe, expect } from "bun:test" import { Cause, Deferred, Effect, Exit, Layer, Option } from "effect" import { GoalLoop, GoalLoopJudgeLLM } from "@/goal/loop" import { Goal } from "@/goal/goal" +import { NotFoundError } from "@/storage/storage" import { GoalEvent } from "@/goal/events" import { EventV2Bridge } from "@/event-v2-bridge" import { SessionStatus } from "@/session/status" @@ -1590,3 +1591,205 @@ describe("GoalLoop — startup scan scoping and hardening (GOAL-FP-01-04 follow- }), ) }) + +// GOAL-FP-01-17 regression suite: every messages-window read in afterIdle +// that fails with storage NotFoundError (session row gone mid-goal, or a +// synthetic session) must degrade to an empty window — never a typed failure +// escaping the fork. Before the fix the pre-judge window escaped into the +// fork's catch and left the goal permanently "active" with zero logs. +describe("GoalLoop — NotFoundError messages window pauses instead of stalling (GOAL-FP-01-17)", () => { + let judgeCalls = 0 + const sessionMock = Layer.mock(Session.Service, { + messages: () => Effect.fail(new NotFoundError({ message: "Session not found" })), + }) + const promptMock = Layer.mock(SessionPrompt.Service, { + // The pause branch only delivers a noReply transcript line; die() proves + // the pause happened without needing a cast for a full WithParts value. + prompt: () => Effect.die(new Error("unreachable outside the pause branch")), + }) + const providerMock = Layer.mock(Provider.Service, {}) + const judgeMock = Layer.succeed( + GoalLoopJudgeLLM, + GoalLoopJudgeLLM.of({ + call: () => + Effect.sync(() => { + judgeCalls += 1 + return JSON.stringify({ verdict: "done", reason: "must never run" }) + }), + }), + ) + + const nfLayer = GoalLoop.layer.pipe( + Layer.provide(sessionMock), + Layer.provide(promptMock), + Layer.provide(providerMock), + Layer.provide(judgeMock), + Layer.provideMerge(Goal.defaultLayer), + Layer.provide(SessionStatus.defaultLayer), + Layer.provideMerge(EventV2Bridge.defaultLayer), + ) + const it = testEffect(nfLayer) + + it.instance("idle with a NotFoundError messages window pauses the goal visibly", () => + Effect.gen(function* () { + judgeCalls = 0 + const loop = yield* GoalLoop.Service + const goal = yield* Goal.Service + const events = yield* EventV2Bridge.Service + + yield* loop.init() + const sid = SessionID.descending() + yield* goal.set(sid, "vanished-session tolerance", 5) + yield* Effect.yieldNow + + yield* events.publish(SessionStatus.Event.Status, { sessionID: sid, status: { type: "idle" } }) + + const paused = yield* pollWithTimeout( + Effect.gen(function* () { + const g = yield* goal.load(sid) + return g && g.status === "paused" ? g : undefined + }), + "goal never paused after a NotFoundError messages window — typed failure escaped the fork", + "5 seconds", + ) + expect(paused.paused_reason).toContain("无 assistant 回复") + // The window must short-circuit before the judge ever runs. + expect(judgeCalls).toBe(0) + }), + ) +}) + +// GOAL-FP-01-18: the post-judge reload window (freshMsgs) needs the same +// tolerance — a session row deleted during the 5-30s judge latency must not +// stall a goal whose turn already committed. The evaluation proceeds to the +// continuation branch (shouldPreempt is defensively false on an empty window). +describe("GoalLoop — NotFoundError on the post-judge reload must not stall (GOAL-FP-01-18)", () => { + let messageCall = 0 + const sessionMock = Layer.mock(Session.Service, { + messages: () => + Effect.suspend(() => { + messageCall += 1 + return messageCall === 1 + ? Effect.succeed([mkAssistant()]) + : Effect.fail(new NotFoundError({ message: "Session not found" })) + }), + }) + const promptMock = Layer.mock(SessionPrompt.Service, { + prompt: () => Effect.die(new Error("unreachable - paused branch not expected")), + prepareIfIdle: () => Effect.succeed(Option.none()), + }) + const providerMock = Layer.mock(Provider.Service, {}) + const judgeMock = Layer.succeed( + GoalLoopJudgeLLM, + GoalLoopJudgeLLM.of({ + call: () => Effect.succeed(JSON.stringify({ verdict: "continue", reason: "more work" })), + }), + ) + + const reloadLayer = GoalLoop.layer.pipe( + Layer.provide(sessionMock), + Layer.provide(promptMock), + Layer.provide(providerMock), + Layer.provide(judgeMock), + Layer.provideMerge(Goal.defaultLayer), + Layer.provide(SessionStatus.defaultLayer), + Layer.provideMerge(EventV2Bridge.defaultLayer), + ) + const it = testEffect(reloadLayer) + + it.instance("a vanished session during judge still commits the turn", () => + Effect.gen(function* () { + messageCall = 0 + const loop = yield* GoalLoop.Service + const goal = yield* Goal.Service + const events = yield* EventV2Bridge.Service + + yield* loop.init() + const sid = SessionID.descending() + yield* goal.set(sid, "post-judge reload tolerance", 5) + yield* Effect.yieldNow + + yield* events.publish(SessionStatus.Event.Status, { sessionID: sid, status: { type: "idle" } }) + + const committed = yield* pollWithTimeout( + Effect.gen(function* () { + const g = yield* goal.load(sid) + return g && g.turns_used >= 1 ? g : undefined + }), + "turn never committed — the post-judge reload failure escaped", + "5 seconds", + ) + expect(committed.turns_used).toBe(1) + expect(committed.status).toBe("active") + expect(messageCall).toBeGreaterThanOrEqual(2) + }), + ) +}) + +// GOAL-FP-01-18b: the judge chain must survive DEFECTS, not just typed +// failures. The production callLLM path (provider.defaultModel → small-model +// resolution → getLanguage → generateText) can defect (config orDie, payload +// decode throws); a defect escaping into the fork was the invisible 0-turn +// stall class. catchCause folds it into the parseFailed budget so the loop +// commits the turn and auto-pauses after MAX_CONSECUTIVE_PARSE_FAILURES. +describe("GoalLoop — judge-chain defect degrades into the parse budget (GOAL-FP-01-18b)", () => { + let judgeCalls = 0 + const sessionMock = Layer.mock(Session.Service, { + messages: () => Effect.succeed([mkAssistant()]), + }) + const promptMock = Layer.mock(SessionPrompt.Service, { + prompt: () => Effect.die(new Error("unreachable - paused branch not expected")), + prepareIfIdle: () => Effect.succeed(Option.none()), + }) + const providerMock = Layer.mock(Provider.Service, {}) + const judgeMock = Layer.succeed( + GoalLoopJudgeLLM, + GoalLoopJudgeLLM.of({ + call: () => + Effect.sync(() => { + judgeCalls += 1 + }).pipe(Effect.flatMap(() => Effect.die(new Error("simulated provider-chain defect")))), + }), + ) + + const defectLayer = GoalLoop.layer.pipe( + Layer.provide(sessionMock), + Layer.provide(promptMock), + Layer.provide(providerMock), + Layer.provide(judgeMock), + Layer.provideMerge(Goal.defaultLayer), + Layer.provide(SessionStatus.defaultLayer), + Layer.provideMerge(EventV2Bridge.defaultLayer), + ) + const it = testEffect(defectLayer) + + it.instance("a defecting judge commits the turn and counts a parse failure", () => + Effect.gen(function* () { + judgeCalls = 0 + const loop = yield* GoalLoop.Service + const goal = yield* Goal.Service + const events = yield* EventV2Bridge.Service + + yield* loop.init() + const sid = SessionID.descending() + yield* goal.set(sid, "judge defect tolerance", 5) + yield* Effect.yieldNow + + yield* events.publish(SessionStatus.Event.Status, { sessionID: sid, status: { type: "idle" } }) + + const committed = yield* pollWithTimeout( + Effect.gen(function* () { + const g = yield* goal.load(sid) + return g && g.turns_used >= 1 ? g : undefined + }), + "turn never committed — the judge defect escaped the fork", + "5 seconds", + ) + expect(judgeCalls).toBe(1) + expect(committed.turns_used).toBe(1) + expect(committed.consecutive_parse_failures).toBe(1) + // First defect is a blip: verdict stays continue, goal keeps running. + expect(committed.status).toBe("active") + }), + ) +}) diff --git a/packages/opencode/test/lib/effect.ts b/packages/opencode/test/lib/effect.ts index 2659ef2743..e178035b42 100644 --- a/packages/opencode/test/lib/effect.ts +++ b/packages/opencode/test/lib/effect.ts @@ -5,7 +5,6 @@ import * as Scope from "effect/Scope" import * as TestClock from "effect/testing/TestClock" import * as TestConsole from "effect/testing/TestConsole" import { memoMap } from "@opencode-ai/core/effect/memo-map" -import type { Config } from "@/config/config" import { TestInstance, withTmpdirInstance } from "../fixture/fixture" import { InstanceStore } from "@/project/instance-store" diff --git a/packages/opencode/test/server/httpapi-event.test.ts b/packages/opencode/test/server/httpapi-event.test.ts index 2b6be2ab6a..010136c93e 100644 --- a/packages/opencode/test/server/httpapi-event.test.ts +++ b/packages/opencode/test/server/httpapi-event.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect } from "bun:test" -import { Effect, Layer, Queue, Schema, Stream } from "effect" +import { Effect, Queue, Schema, Stream } from "effect" import { EventPaths } from "../../src/server/routes/instance/httpapi/groups/event" import { resetDatabase } from "../fixture/db" import { disposeAllInstances, TestInstance } from "../fixture/fixture" diff --git a/packages/opencode/test/server/session-messages.test.ts b/packages/opencode/test/server/session-messages.test.ts index 3e66c59d5e..05bf5d0761 100644 --- a/packages/opencode/test/server/session-messages.test.ts +++ b/packages/opencode/test/server/session-messages.test.ts @@ -3,7 +3,6 @@ import { SessionV1 } from "@opencode-ai/core/v1/session" import { Effect, Layer } from "effect" import { HttpClientResponse } from "effect/unstable/http" import { Session as SessionNs } from "@/session/session" -import { MessageV2 } from "../../src/session/message-v2" import { MessageID, PartID, type SessionID } from "../../src/session/schema" import { disposeAllInstances, TestInstance } from "../fixture/fixture" diff --git a/packages/opencode/test/session/structured-output-integration.test.ts b/packages/opencode/test/session/structured-output-integration.test.ts index 319b3bd728..9010d52c67 100644 --- a/packages/opencode/test/session/structured-output-integration.test.ts +++ b/packages/opencode/test/session/structured-output-integration.test.ts @@ -4,7 +4,6 @@ import { Ripgrep } from "@opencode-ai/core/ripgrep" import { Effect, Layer } from "effect" import { Session } from "@/session/session" import { SessionPrompt } from "../../src/session/prompt" -import { MessageV2 } from "../../src/session/message-v2" import { testEffect } from "../lib/effect" // Skip tests if no API key is available diff --git a/packages/opencode/test/session/structured-output.test.ts b/packages/opencode/test/session/structured-output.test.ts index f71b535a9d..d79a4538a7 100644 --- a/packages/opencode/test/session/structured-output.test.ts +++ b/packages/opencode/test/session/structured-output.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test" import { SessionV1 } from "@opencode-ai/core/v1/session" import { Exit, Schema } from "effect" -import { MessageV2 } from "../../src/session/message-v2" import { SessionPrompt } from "../../src/session/prompt" import { SessionID, MessageID } from "../../src/session/schema" diff --git a/packages/opencode/test/tool/grep.test.ts b/packages/opencode/test/tool/grep.test.ts index da314d7bf3..94bbe30a99 100644 --- a/packages/opencode/test/tool/grep.test.ts +++ b/packages/opencode/test/tool/grep.test.ts @@ -5,7 +5,7 @@ import os from "os" import path from "path" import { Effect, Layer } from "effect" import { GrepTool } from "../../src/tool/grep" -import { provideInstance, testInstanceStoreLayer, TestInstance, tmpdirScoped } from "../fixture/fixture" +import { provideInstance, testInstanceStoreLayer, TestInstance } from "../fixture/fixture" import { SessionID, MessageID } from "../../src/session/schema" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { Global } from "@opencode-ai/core/global" diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index 67205f56e3..223d1a1dcf 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -5,7 +5,6 @@ import path from "path" import { Agent } from "../../src/agent/agent" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import { FSUtil } from "@opencode-ai/core/fs-util" -import { Global } from "@opencode-ai/core/global" import { Config } from "@/config/config" import { RuntimeFlags } from "@/effect/runtime-flags" import { Ripgrep } from "@opencode-ai/core/ripgrep" diff --git a/packages/opencode/test/tool/skill.test.ts b/packages/opencode/test/tool/skill.test.ts index 177dbddcbb..8baff8a0ad 100644 --- a/packages/opencode/test/tool/skill.test.ts +++ b/packages/opencode/test/tool/skill.test.ts @@ -4,7 +4,6 @@ import { Ripgrep } from "@opencode-ai/core/ripgrep" import { Cause, Effect, Exit, Layer } from "effect" import { afterEach, describe, expect } from "bun:test" import path from "path" -import type { Permission } from "../../src/permission" import type { Tool } from "@/tool/tool" import { SkillTool } from "../../src/tool/skill" import { ToolRegistry } from "@/tool/registry" diff --git a/packages/opencode/test/tool/truncation.test.ts b/packages/opencode/test/tool/truncation.test.ts index 119e41062c..0879cbbe22 100644 --- a/packages/opencode/test/tool/truncation.test.ts +++ b/packages/opencode/test/tool/truncation.test.ts @@ -4,7 +4,6 @@ import { NodeFileSystem } from "@effect/platform-node" import { FSUtil } from "@opencode-ai/core/fs-util" import { Effect, FileSystem, Layer } from "effect" import { Truncate } from "@/tool/truncate" -import { Config } from "@/config/config" import { Identifier } from "../../src/id/id" import { Process } from "@/util/process" import path from "path" From 04492b479a95418199da6a3bc8160f9d05fb9f55 Mon Sep 17 00:00:00 2001 From: Lex Date: Sun, 16 Aug 2026 03:15:33 +0800 Subject: [PATCH 2/2] fix(goal): retry ESC pause persistence before giving up the goal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transient DB failure during pauseForUserCancel would silently lose the pause: the turnDriven mark cleared, the pause never persisted, and the next idle event resurrected the goal against the user's explicit ESC (shouldPreempt cannot catch ESC — it adds no user message). Retry the pause up to twice with 50ms backoff (Effect.exit captures defects, unlike typed retry); on exhausted retries log loudly instead of warning, so a resurrecting goal is never invisible. No clean fault-injection seam for the retry branch (test DB is :memory:, pauseAndPublish is a layer closure over drizzle orDie) — happy path covered by test/goal/turn-scope.test.ts; seam absence recorded per diagnosis policy. --- packages/opencode/src/goal/goal.ts | 43 +++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/opencode/src/goal/goal.ts b/packages/opencode/src/goal/goal.ts index 106d0be3cc..dc3fbe8fb4 100644 --- a/packages/opencode/src/goal/goal.ts +++ b/packages/opencode/src/goal/goal.ts @@ -1,6 +1,6 @@ export * as Goal from "./goal" -import { Effect, Layer, Context, Schema, Fiber } from "effect" +import { Effect, Layer, Context, Schema, Fiber, Cause, Exit } from "effect" import { desc, eq, sql } from "drizzle-orm" import { SessionTable } from "@opencode-ai/core/session/sql" import { LayerNode } from "@opencode-ai/core/effect/layer-node" @@ -227,22 +227,39 @@ const serviceLayer = Layer.effect( return GoalPrompts.GOAL_TURN_MAX_STEPS }) - // ESC-on-goal-turn: durable pause + lease release + mark clear, failure- - // absorbed. Called from SessionPrompt.cancel so a user ESC on a goal turn - // pauses the goal instead of letting the post-cancel idle event resurrect - // it with an unwanted continuation. + // ESC-on-goal-turn: durable pause + lease release + mark clear. Called + // from SessionPrompt.cancel so a user ESC on a goal turn pauses the goal + // instead of letting the post-cancel idle event resurrect it with an + // unwanted continuation. + // + // GOAL-FP-01-19: a transient DB failure must not silently lose the pause + // — the mark would clear, the pause never persist, and the next idle + // would resurrect the goal against the user's explicit intent + // (shouldPreempt cannot catch it: ESC adds no user message). Retry the + // pause twice with a short backoff; if it still fails, log LOUDLY — the + // goal may resurrect, but it will never do so invisibly. const pauseForUserCancel = Effect.fnUntraced(function* (sessionID: SessionID, reason: string) { - const paused = yield* pauseAndPublish(sessionID, reason).pipe( - Effect.catchCause((cause) => - Effect.logWarning("goal pause on cancel failed", { sessionID, cause: String(cause) }).pipe( - Effect.as(undefined), - ), - ), - ) - if (paused) + let paused: GoalState.Info | undefined + let lastCause: Cause.Cause | undefined + for (let attempt = 0; attempt < 3; attempt++) { + const exit = yield* pauseAndPublish(sessionID, reason).pipe(Effect.exit) + if (Exit.isSuccess(exit)) { + paused = exit.value + break + } + lastCause = exit.cause + if (attempt < 2) yield* Effect.sleep("50 millis") + } + if (paused) { yield* automation.unregister(sessionID, { kind: "goal", id: paused.goal_id ?? "legacy" }).pipe( Effect.ignore, ) + } else { + yield* Effect.logError( + "goal pause on cancel failed after retries — goal may resurrect on next idle", + { sessionID, cause: lastCause ? Cause.pretty(lastCause) : "unknown" }, + ) + } turnDriven.delete(sessionID) return paused })