Skip to content

Commit facd7c1

Browse files
committed
fix(retry): use plain Cause.done + Pull.isDoneCause
Both reviewers (spec + quality, codex-5.3) flagged that retry.ts:129/131 used Effect.failCause(Cause.Done(...) as unknown as Cause.Cause<number>) with double casts to terminate the schedule. Plan A.2.2 specifies 'return Cause.done(meta.attempt)' directly; the cast pattern was hiding a type mismatch and making the control flow harder to reason about. - retry.ts: replace both Effect.failCause casts with plain Cause.done(meta.attempt) — the Effect<never, Done<number>> that fromStepWithMetadata already expects as a stop signal. - retry.test.ts: my original A.2.1 assertion used Cause.isDone(cause), which takes a Done value and returned false on a Cause<Done>. Switch to Pull.isDoneCause(cause), which correctly recognizes a Cause whose reason is a Done marker. Drop unused Cause import. All 54 targeted tests pass, typecheck clean. Addresses spec+quality review of 2176730.
1 parent 2176730 commit facd7c1

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

packages/opencode/src/session/retry.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ export function policy(opts: {
126126
const error = opts.parse(meta.input)
127127
const message = retryable(error)
128128
const transport = transportMessage(error)
129-
if (!message) return Effect.failCause(Cause.Done(meta.attempt) as unknown as Cause.Cause<number>)
129+
if (!message) return Cause.done(meta.attempt)
130130
if (transport && !MessageV2.APIError.isInstance(error) && meta.attempt > TRANSPORT_RETRY_CAP) {
131-
return Effect.failCause(Cause.Done(meta.attempt) as unknown as Cause.Cause<number>)
131+
return Cause.done(meta.attempt)
132132
}
133133
return Effect.gen(function* () {
134134
const wait = delay(meta.attempt, MessageV2.APIError.isInstance(error) ? error : undefined)

packages/opencode/test/session/retry.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"
22
import type { NamedError } from "@opencode-ai/shared/util/error"
33
import { APICallError } from "ai"
44
import { setTimeout as sleep } from "node:timers/promises"
5-
import { Cause, Effect, Exit, Layer, Schedule } from "effect"
5+
import { Effect, Exit, Layer, Pull, Schedule } from "effect"
66
import { SessionRetry } from "../../src/session/retry"
77
import { MessageV2 } from "../../src/session/message-v2"
88
import { SSEStallError } from "../../src/provider/provider"
@@ -307,9 +307,10 @@ describe("SessionRetry.policy — transport retry budget", () => {
307307
expect(setCalls).toBe(5)
308308
expect(terminal).toBeDefined()
309309
if (!Exit.isFailure(terminal!)) throw new Error("expected terminal exit to be Failure")
310-
// Policy signals "stop retrying" via Cause.done(n), not Cause.fail.
311-
// Cause.isDone confirms schedule completed normally, not crashed.
312-
expect(Cause.isDone(terminal.cause)).toBe(true)
310+
// Policy signals "stop retrying" via Cause.done(n), surfaced by Pull
311+
// as a Failure whose cause contains a Done reason. Pull.isDoneCause
312+
// confirms schedule completed normally (not crashed via fail/die).
313+
expect(Pull.isDoneCause(terminal.cause)).toBe(true)
313314
}),
314315
)
315316
})

0 commit comments

Comments
 (0)