Skip to content

Commit d80f0fa

Browse files
committed
fix(tui): keep history retries scoped to the initial read
1 parent 259b385 commit d80f0fa

3 files changed

Lines changed: 121 additions & 61 deletions

File tree

‎packages/tui/src/routes/session/rows.ts‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,18 @@ export function createSessionRows(
130130
onCleanup(() => abort(controller))
131131
rebuild()
132132
void data.session.pending.sync(id).catch(() => undefined)
133-
void retrySessionSync(async () => {
134-
await data.session.message.sync(id)
135-
if (controller.signal.aborted) return
136-
rebuild()
137-
// Restoration waits for complete boundary groups so saved group IDs resolve.
138-
await completeGroupBoundary({
139-
rows,
140-
messages: () => data.session.message.list(id).length,
141-
more: () => data.session.message.more(id),
142-
loadMore: () => data.session.message.loadMore(id),
143-
active: () => !controller.signal.aborted && sessionID() === id,
144-
})
145-
}, controller.signal)
146-
.then(() => {
133+
void retrySessionSync(() => data.session.message.sync(id), controller.signal)
134+
.then(async () => {
135+
if (controller.signal.aborted) return
136+
rebuild()
137+
// Restoration waits for complete boundary groups so saved group IDs resolve.
138+
await completeGroupBoundary({
139+
rows,
140+
messages: () => data.session.message.list(id).length,
141+
more: () => data.session.message.more(id),
142+
loadMore: () => data.session.message.loadMore(id),
143+
active: () => !controller.signal.aborted && sessionID() === id,
144+
}).catch(() => undefined)
147145
if (!controller.signal.aborted) onSynced?.(id)
148146
})
149147
.catch((error) => {

‎packages/tui/test/session-resume-retry.test.tsx‎

Lines changed: 36 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,25 @@ const session = {
1616
time: { created: 0, updated: 0 },
1717
}
1818
const location = { directory, project: { id: "project", directory, canonical: directory } }
19+
const assistant = {
20+
id: "message-2",
21+
type: "assistant",
22+
agent: "build",
23+
model: { providerID: "demo", id: "demo-model" },
24+
content: [{ type: "text", text: "History recovered" }],
25+
finish: "stop",
26+
time: { created: 2, completed: 3 },
27+
}
1928
const messages = [
2029
{ id: "message-1", type: "user", text: "Restore this history", time: { created: 1 } },
21-
{
22-
id: "message-2",
23-
type: "assistant",
24-
agent: "build",
25-
model: { providerID: "demo", id: "demo-model" },
26-
content: [{ type: "text", text: "History recovered" }],
27-
finish: "stop",
28-
time: { created: 2, completed: 3 },
29-
},
30+
assistant,
3031
]
3132

32-
async function launch(options: { target: Read; failures?: number; status?: number; block?: boolean }) {
33+
async function launch(options: { target: Read; failures?: number; status?: number; boundary?: boolean }) {
3334
const reads = { session: 0, message: 0 }
34-
const release = Promise.withResolvers<void>()
3535
const setup = await createAppFixture({
3636
args: { sessionID: session.id },
37-
config: { animations: false, tabs: { mode: "off" }, keybinds: { "session.new": "f6" } },
37+
config: { animations: false, tabs: { mode: "off" } },
3838
fetch: async (url) => {
3939
if (url.pathname === "/api/agent")
4040
return json({ location, data: [{ id: "build", mode: "primary", hidden: false, permissions: [] }] })
@@ -51,32 +51,31 @@ async function launch(options: { target: Read; failures?: number; status?: numbe
5151
: undefined
5252
if (read) {
5353
reads[read]++
54-
if (options.block && read === options.target) await release.promise
54+
if (read === "message" && url.searchParams.has("cursor"))
55+
return json({ message: "older page unavailable" }, { status: 401 })
5556
if (read === options.target && reads[read] <= (options.failures ?? 2)) {
5657
const status = options.status ?? 503
5758
return json(
5859
status === 404
5960
? { _tag: "SessionNotFoundError", sessionID: session.id, message: `Session not found: ${session.id}` }
60-
: { message: "temporarily unavailable" },
61+
: { message: status === 401 ? "history access denied" : "temporarily unavailable" },
6162
{ status },
6263
)
6364
}
64-
return json(read === "session" ? { data: session } : { data: messages.toReversed(), cursor: {} })
65+
if (read === "session") return json({ data: session })
66+
return json({
67+
data: options.boundary
68+
? [{ ...assistant, content: [{ type: "reasoning", text: "Existing thought" }, ...assistant.content] }]
69+
: messages.toReversed(),
70+
cursor: options.boundary ? { next: "older" } : {},
71+
})
6572
}
6673
if (url.pathname === `/api/session/${session.id}/inbox`) return json({ data: [] })
6774
return undefined
6875
},
6976
})
7077
await setup.ready
71-
return {
72-
...setup,
73-
reads,
74-
release: release.resolve,
75-
async [Symbol.asyncDispose]() {
76-
release.resolve()
77-
await setup[Symbol.asyncDispose]()
78-
},
79-
}
78+
return { ...setup, reads }
8079
}
8180

8281
async function wait(condition: () => boolean) {
@@ -108,32 +107,22 @@ test("a missing session reports the error and returns to the actual home screen
108107
expect(setup.captureCharFrame()).not.toContain("Restore this history")
109108
})
110109

111-
// The shared retry tests control time and verify each delay. This case exercises the complete
112-
// app's real 7.5-second retry budget and the user-visible error after all five reads fail.
113-
test("exhausted history reads report an error instead of silently leaving an empty transcript", async () => {
114-
await using setup = await launch({ target: "message", failures: Infinity })
115-
await wait(() => setup.captureCharFrame().includes("Failed to load session history"))
116-
expect(setup.reads.message).toBe(5)
117-
expect(setup.captureCharFrame()).toContain("UnexpectedStatus: 503")
110+
test("a terminal history error is reported without retrying", async () => {
111+
await using setup = await launch({ target: "message", failures: Infinity, status: 401 })
112+
await setup.waitForFrame(
113+
(frame) => frame.includes("Failed to load session history") && frame.includes("history access denied"),
114+
)
115+
expect(setup.reads.message).toBe(1)
118116
expect(setup.captureCharFrame()).not.toContain(logo.left[1])
119117
})
120118

121-
test.each(["session", "message"] satisfies Read[])(
122-
"leaving during an in-flight %s read cancels stale recovery",
123-
async (target) => {
124-
await using setup = await launch({ target, block: true, failures: Infinity })
125-
await wait(() => setup.reads[target] === 1)
126-
setup.mockInput.pressKey("F6")
127-
await setup.waitForFrame((frame) => frame.includes(logo.left[1]))
128-
setup.release()
129-
// Cross the first retry boundary to catch an obsolete read starting after the route was left.
130-
await Bun.sleep(600)
131-
expect(setup.reads[target]).toBe(1)
132-
expect(setup.captureCharFrame()).toContain(logo.left[1])
133-
expect(setup.captureCharFrame()).not.toContain("UnexpectedStatus")
134-
expect(setup.captureCharFrame()).not.toContain("Failed to load session history")
135-
},
136-
)
119+
test("an older-page failure does not report an initial history error", async () => {
120+
await using setup = await launch({ target: "message", failures: 0, boundary: true })
121+
await wait(() => setup.reads.message === 2)
122+
await waitForHighlightedFrame(setup, (frame) => frame.includes("History recovered"))
123+
expect(setup.captureCharFrame()).not.toContain("Failed to load session history")
124+
expect(setup.captureCharFrame()).not.toContain("UnexpectedStatus")
125+
})
137126

138127
test("reconnecting restarts a failed history read without stale retries or errors", async () => {
139128
await using setup = await launch({ target: "message", failures: 1 })

‎packages/tui/test/util/session-sync.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,76 @@ test.each([
6464
jest.useRealTimers()
6565
}
6666
})
67+
68+
test("exhausts five reads after four delays without a final backoff", async () => {
69+
jest.useFakeTimers()
70+
const controller = new AbortController()
71+
const error = new ClientError("UnexpectedStatus", { cause: { status: 503 } })
72+
const attempts: number[] = []
73+
try {
74+
const task = retrySessionSync(async () => {
75+
attempts.push(Date.now())
76+
throw error
77+
}, controller.signal).catch((error) => error)
78+
await Promise.resolve()
79+
await Promise.resolve()
80+
for (const delay of [500, 1000, 2000, 4000]) {
81+
jest.advanceTimersByTime(delay)
82+
await Promise.resolve()
83+
await Promise.resolve()
84+
}
85+
expect(await task).toBe(error)
86+
expect(attempts.map((time) => time - attempts[0])).toEqual([0, 500, 1500, 3500, 7500])
87+
expect(jest.getTimerCount()).toBe(0)
88+
} finally {
89+
controller.abort()
90+
jest.useRealTimers()
91+
}
92+
})
93+
94+
test("cancelling a read clears its pending retry", async () => {
95+
jest.useFakeTimers()
96+
const controller = new AbortController()
97+
const reason = new Error("session left")
98+
let calls = 0
99+
try {
100+
const task = retrySessionSync(async () => {
101+
calls++
102+
throw new ClientError("UnexpectedStatus", { cause: { status: 503 } })
103+
}, controller.signal).catch((error) => error)
104+
await Promise.resolve()
105+
await Promise.resolve()
106+
expect(jest.getTimerCount()).toBe(1)
107+
controller.abort(reason)
108+
expect(await task).toBe(reason)
109+
expect(jest.getTimerCount()).toBe(0)
110+
jest.runAllTimers()
111+
expect(calls).toBe(1)
112+
} finally {
113+
controller.abort()
114+
jest.useRealTimers()
115+
}
116+
})
117+
118+
test("leaving during an in-flight read prevents a stale retry", async () => {
119+
jest.useFakeTimers()
120+
const controller = new AbortController()
121+
const response = Promise.withResolvers<void>()
122+
const reason = new Error("session left")
123+
let calls = 0
124+
try {
125+
const task = retrySessionSync(() => {
126+
calls++
127+
return response.promise
128+
}, controller.signal).catch((error) => error)
129+
controller.abort(reason)
130+
response.reject(new ClientError("UnexpectedStatus", { cause: { status: 503 } }))
131+
expect(await task).toBe(reason)
132+
expect(jest.getTimerCount()).toBe(0)
133+
jest.runAllTimers()
134+
expect(calls).toBe(1)
135+
} finally {
136+
controller.abort()
137+
jest.useRealTimers()
138+
}
139+
})

0 commit comments

Comments
 (0)