cloudflare / cloudflare/agents

Think reports a recovered turn as completed when final assistant message persistence fails

Open
#1,997 1 comment 0 reactions 1 assignee Claimed by @threepointone View on GitHub
bug think
Dominant language
TypeScript
Stars
5.6k
Forks
711
Avg merge
1d 20h
Merged PRs (30d)
53

Description

## Package versions

- `@cloudflare/think`: `0.15.1`
- `agents`: `0.20.1`
- Session storage: `PostgresSessionProvider`
- Connection: `pg.Client` through Cloudflare Hyperdrive

## Summary

When the model stream completes but persisting the final assistant message throws, Think logs the persistence error and still returns `status: "completed"`.

If this happens during chat recovery, the recovery incident is marked completed even though the generated assistant message was not saved in the Session.

The result is a successfully generated response that disappears from the transcript. After reconnecting or reloading, the user cannot see the answer or continue from it.

## Observed sequence

1. The original browser response stream disconnected.
2. Think detected the interrupted turn and scheduled a continuation.
3. The continuation successfully generated the remaining response.
4. Final assistant persistence failed inside `PostgresSessionProvider.getMessage()`.
5. Think logged `Failed to persist assistant message`.
6. The turn emitted `status: "completed"`.
7. The recovery incident was marked completed.
8. The final assistant response was absent from the persisted transcript.

There was no explicit abort or user cancellation.

## Relevant implementation

The final persistence and response hook share a catch block:

https://github.com/cloudflare/agents/blob/main/packages/think/src/think.ts#L12184-L12205

Conceptually:

```ts
try {
await this._persistAssistantMessage(assistantMsg, parentId);
await this._fireResponseHook({ status: "completed" });
} catch (error) {
console.error("Failed to persist assistant message:", error);
}

this._onStreamingTurnFinalized();
return { status: "completed" };
```

The lifecycle documentation says `onChatResponse` runs after the message is persisted and provides `onChatError` with `stage: "persist"`:

https://github.com/cloudflare/agents/blob/main/docs/think/lifecycle-hooks.md

## Expected behavior

A turn should not be reported as completed unless its assistant message has been durably accepted by the configured Session provider.

If final assistant persistence fails, Think should:

1. Preserve the already-generated assistant message.
2. Route the failure through `onChatError` with `stage: "persist"`.
3. Return a non-completed result or schedule a persistence-only recovery.
4. Avoid rerunning inference or settled tool calls merely to retry the database write.
5. Ensure `_onStreamingTurnFinalized()` still runs so the turn lock is released.

At minimum, the persistence exception should not be swallowed and converted into a successful completion.

## Why this matters

The model and tool work has already completed. The remaining operation is committing the generated assistant message.

Treating the turn as completed causes silent transcript loss and prevents recovery from repairing it. Retrying the entire model turn is also undesirable because it can duplicate inference, cost, and side effects.

A persistence-only retry or explicit failed state would preserve the completed response without replaying the turn.

## Local mitigation

We added a `pg.Client`-compatible adapter around `PostgresSessionProvider` that:

- detects connection and transport errors;
- creates a fresh connection;
- retries the identical SQL operation once;
- logs the underlying database errors;
- does not rerun the model or replay the chat turn.

This reduces transient failures but cannot correct Think’s terminal status if the retry also fails, because the final persistence exception is still swallowed internally.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.