cloudwego / cloudwego/eino

adk: ChatModelAgent.Resume panics synchronously on corrupted checkpoint state instead of returning an error

Open
#1,246 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
13k
Forks
1.1k
Avg merge
4h 6m
Merged PRs (30d)
41

Description

## Problem

`ChatModelAgent.Resume` panics synchronously when the persisted checkpoint state is corrupted or has an unexpected shape, instead of surfacing an error.

In v0.9.18 there are four panic branches in `adk/chatmodel.go` inside the `Resume` implementation:

- `chatmodel.go:1582` — `info == nil` → `panic("... was asked to resume but info is nil")`
- `chatmodel.go:1586` — `info.InterruptState == nil` → `panic("... has no state")`
- `chatmodel.go:1591` — `info.InterruptState` is not `[]byte` → `panic("... has invalid interrupt state type: %T")`
- `chatmodel.go:1614` — `info.ResumeData` is not `*ChatModelAgentResumeData` → `panic("... has invalid resume data type: %T")`

## Why this matters

A checkpoint living in an external `CheckPointStore` (Redis, SQL, object storage, ...) can be corrupted outside the process: partial writes, gob/codec changes across upgrades, state written by an older or newer eino version, or manual edits. When that happens, `Resume` takes down the calling goroutine with a panic.

For long-running orchestration services (e.g. Temporal workers resuming many agents concurrently), a single corrupted checkpoint must not crash the worker process. Today every caller has to wrap `Resume` in its own `recover()` to survive — we run eino v0.9.18 pinned in production and had to add exactly such a recover-and-classify shim, which feels like something the framework should offer natively.

Note the asymmetry inside the same function: a `preprocessComposeCheckpoint` failure already returns an error event on the iterator instead of panicking (`chatmodel.go:1600-1606`). The four state-validation branches should follow the same convention.

## Minimal reproduction sketch

```go
// Given any adk.CheckPointStore, persist a checkpoint record whose decoded
// InterruptInfo.InterruptState is nil or not []byte (e.g. gob-encode a
// different concrete type under the checkpoint key, or hand-craft the record).
iter, err := runner.Resume(ctx, corruptedCheckPointID)
// => synchronous panic at chatmodel.go:1591 (or :1586) instead of an error
```

A record whose `ResumeData` holds an unexpected concrete type reaches the type assertion at `chatmodel.go:1614` and panics the same way.

## Expected behavior

All four branches should deliver a typed error through the returned iterator (emit an event with `Err` set and close — the same pattern already used for `preprocessComposeCheckpoint` failures), ideally wrapping an exported sentinel such as `adk.ErrInvalidCheckpointState` so callers can classify "checkpoint unusable" with `errors.Is` and fall back to a fresh run. No panic on the resume path for bad persisted state.

## Environment

- eino version: v0.9.18 (verified against the tagged module source)
- Go: 1.24

Contributor guide

Open the contributing guide

Research direction

Start in adk/chatmodel.go at the Resume implementation and inspect the existing preprocessComposeCheckpoint error path around lines 1600-1606. Trace the four checkpoint-state validation branches and how the returned iterator emits errors. Done means corrupted state produces an error event and closes the iterator without panicking, with callers able to classify the invalid checkpoint state.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
ai, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.