Skip to content

Commit 9e6ab76

Browse files
committed
test(phase-c): address codex review blockers
- Test B: replace poll+await pattern with Fiber.await+timeoutOrElse+Exit.isSuccess to surface defect paths instead of treating poll truthy as success. - Test B: assert RejectedError propagates to the question tool's error output by walking root + child sessions and matching /dismissed/i. - Compress header comment (31 lines -> 9). - Replace `function* ({ llm })` destructuring with `function* (input)` + input.llm.
1 parent 5764aa1 commit 9e6ab76

1 file changed

Lines changed: 48 additions & 54 deletions

File tree

packages/opencode/test/session/subagent-hang-regression.test.ts

Lines changed: 48 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,17 @@
11
// Phase C regression gates for the subagent-hang hardening effort.
22
//
3-
// Two failure modes this file pins down:
4-
//
5-
// 1. SSE stall = indefinite hang. If a provider starts a response and then
6-
// stops sending chunks, the loop used to block forever. Phase A wrapped
7-
// SSE bodies with `wrapSSE`, which raises `SSEStallError` on inter-chunk
8-
// timeout. `SSEStallError` is classified transport-retryable by
9-
// `SessionRetry.retryable` (see src/session/retry.ts:25) so the
10-
// processor's `Effect.retry(SessionRetry.policy(...))` observes it,
11-
// calls `SessionStatus.set({ type: "retry", ... })`, then backs off.
12-
// The `session.error` bus event only fires AFTER retries are exhausted
13-
// (5 transport attempts, 2+4+8+16+30s = 60s of backoff). This test
14-
// therefore gates on the retry transition — if the stall surfaced as
15-
// a terminal error instead, or hung indefinitely without triggering
16-
// retry, this test fails fast.
17-
//
18-
// 2. Subagent question in headless run = deadlock. A subagent that invokes
19-
// the `question` tool publishes `question.asked` and awaits an answer.
20-
// In `opencode run` (headless) there is no interactive client, so
21-
// Phase B added `RunEvents` which subscribes to the Bus and auto-rejects
22-
// descendant questions/permissions. Without that handler the loop
23-
// never returns. RunEvents lives in the CLI layer (see
24-
// `src/cli/cmd/run-events.ts` + `src/cli/cmd/run.ts`); it is NOT wired
25-
// into `SessionPrompt.loop` directly. This test therefore drives the
26-
// loop directly and mounts an in-test subscriber that mirrors the
27-
// RunEvents contract (reject descendant questions, reject permissions).
28-
// That still pins the end-to-end contract — if the Bus events are no
29-
// longer published, or Question.reject no longer unblocks the tool, or
30-
// the task-tool flow no longer propagates subagent completion back to
31-
// the parent, the test fails.
32-
//
33-
// Any change that makes either assertion fail is a regression.
3+
// 1. SSE stall: Phase A's wrapSSE must convert a stalled stream into
4+
// SSEStallError, which SessionRetry classifies as transport-retryable
5+
// and surfaces as a `retry` SessionStatus. Gates against indefinite hangs.
6+
// 2. Subagent question in headless: Phase B's Question→Bus publish +
7+
// Question.reject→Deferred.fail contract must allow an external
8+
// subscriber (mirroring RunEvents) to unblock a subagent question tool.
9+
// Gates against headless deadlock when the user can't answer.
3410

3511
import { NodeFileSystem } from "@effect/platform-node"
3612
import { FetchHttpClient } from "effect/unstable/http"
3713
import { expect } from "bun:test"
38-
import { Effect, Fiber, Layer } from "effect"
14+
import { Effect, Exit, Fiber, Layer } from "effect"
3915
import { Agent as AgentSvc } from "../../src/agent/agent"
4016
import { Bus } from "../../src/bus"
4117
import { Command } from "../../src/command"
@@ -252,7 +228,7 @@ it.live(
252228
"SSE stall triggers retry, not indefinite hang",
253229
() =>
254230
provideTmpdirServer(
255-
Effect.fnUntraced(function* ({ llm }) {
231+
Effect.fnUntraced(function* (input) {
256232
const prompt = yield* SessionPrompt.Service
257233
const sessions = yield* Session.Service
258234
const sessionStatus = yield* SessionStatus.Service
@@ -261,7 +237,7 @@ it.live(
261237
// sends another frame. With chunkTimeout=1000ms the loop's wrapSSE
262238
// fires SSEStallError after ~1s, which the retry schedule catches
263239
// and converts into a status transition.
264-
yield* llm.push(reply().hang().item())
240+
yield* input.llm.push(reply().hang().item())
265241

266242
const chat = yield* sessions.create({
267243
title: "SSE stall",
@@ -310,7 +286,7 @@ it.live(
310286
"subagent question in headless run does not deadlock",
311287
() =>
312288
provideTmpdirServer(
313-
Effect.fnUntraced(function* ({ llm }) {
289+
Effect.fnUntraced(function* (input) {
314290
const prompt = yield* SessionPrompt.Service
315291
const sessions = yield* Session.Service
316292
const bus = yield* Bus.Service
@@ -319,15 +295,15 @@ it.live(
319295
const sessionStatus = yield* SessionStatus.Service
320296

321297
// Reply 1 (root): dispatch the task tool to spawn a subagent.
322-
yield* llm.tool("task", {
298+
yield* input.llm.tool("task", {
323299
description: "ask the user",
324300
prompt: "use the question tool to ask the user",
325301
subagent_type: "general",
326302
})
327303
// Reply 2 (subagent): call the question tool. Our bus subscriber
328304
// mirrors the RunEvents contract and rejects this question, which
329305
// unblocks the subagent's question tool with RejectedError.
330-
yield* llm.tool("question", {
306+
yield* input.llm.tool("question", {
331307
questions: [
332308
{
333309
question: "proceed?",
@@ -383,28 +359,46 @@ it.live(
383359

384360
const fiber = yield* prompt.loop({ sessionID: chat.id }).pipe(Effect.forkChild)
385361

386-
// Primary gate: the root fiber must complete in bounded time. If the
387-
// subagent's question tool were left blocked on an unanswered
388-
// deferred, this poll would never see the fiber finish. 10s upper
389-
// bound — the happy-path finish is well under a second.
390-
yield* Effect.promise(async () => {
391-
const end = Date.now() + 10_000
392-
while (Date.now() < end) {
393-
if (fiber.pollUnsafe()) return
394-
await new Promise((done) => setTimeout(done, 25))
395-
}
396-
throw new Error("root loop did not complete within 10s — subagent question likely deadlocked")
397-
})
362+
// Primary gate: the root fiber must complete in bounded time and
363+
// succeed. Join under a 10s timeout that fails with a clear message
364+
// if the loop hangs. Exit check guards against silent defect paths —
365+
// a passing `pollUnsafe()` truthy check could miss these.
366+
const exit = yield* Fiber.await(fiber).pipe(
367+
Effect.timeoutOrElse({
368+
duration: "10 seconds",
369+
orElse: () => Effect.die(new Error("root loop did not complete within 10s — subagent question likely deadlocked")),
370+
}),
371+
)
372+
expect(Exit.isSuccess(exit)).toBe(true)
398373

399-
// Fiber completed. The subagent's question tool should have been
400-
// rejected at least once — that is the whole Phase B contract under
401-
// test.
374+
// Phase B contract: the subagent's question tool must have been
375+
// rejected at least once via the bus subscriber.
402376
expect(questionsRejected).toBeGreaterThanOrEqual(1)
377+
378+
// The rejection must propagate into the subagent's tool output so
379+
// the parent (task tool) sees the failure. Walk the root + child
380+
// sessions and locate the question tool part — it must be in error
381+
// state with the RejectedError message.
382+
const children = yield* sessions.children(chat.id)
383+
const allSessionIDs = [chat.id, ...children.map((c) => c.id)]
384+
const questionErrors: string[] = []
385+
for (const sid of allSessionIDs) {
386+
const messages = yield* sessions.messages({ sessionID: sid })
387+
for (const msg of messages) {
388+
for (const part of msg.parts) {
389+
if (part.type === "tool" && part.tool === "question" && part.state.status === "error") {
390+
questionErrors.push(part.state.error)
391+
}
392+
}
393+
}
394+
}
395+
expect(questionErrors.length).toBeGreaterThanOrEqual(1)
396+
// Question.RejectedError.message => "The user dismissed this question".
397+
expect(questionErrors.some((e) => /dismissed/i.test(e))).toBe(true)
398+
403399
// And the root session should settle idle (not stuck busy).
404400
const finalStatus = yield* sessionStatus.get(chat.id)
405401
expect(finalStatus.type).toBe("idle")
406-
407-
yield* Fiber.await(fiber)
408402
}),
409403
{ git: true, config: (url) => ({ ...providerCfg(url), agent: { general: { permission: { question: "allow" } } } }) },
410404
),

0 commit comments

Comments
 (0)