adk: ChatModelAgent.Resume panics synchronously on corrupted checkpoint state instead of returning an error
- 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
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