Skip to content

fix(sdk): handle cyclic run-state on session resume - #945

Open
KazenDev wants to merge 1 commit into
CodebuffAI:mainfrom
KazenDev:fix/run-state-cycle
Open

fix(sdk): handle cyclic run-state on session resume#945
KazenDev wants to merge 1 commit into
CodebuffAI:mainfrom
KazenDev:fix/run-state-cycle

Conversation

@KazenDev

@KazenDev KazenDev commented Aug 5, 2026

Copy link
Copy Markdown

Fixes a crash surfaced while testing #944 (the /undo and /redo feature):
on the second interaction of a resumed session, the CLI died with

Error: JSON.stringify cannot serialize cyclic structures.

Root cause

applyOverridesToSessionState in sdk/src/run-state.ts deep-clones the
session state with a JSON round-trip (JSON.parse(JSON.stringify(...))).
The run state can carry cyclic values — e.g. recursive zod lazy schemas
inside tool blocks — and JSON.stringify throws on cycles. So whenever a
saved session is resumed (freebuff --continue) while its run state holds
such a schema, the resume path crashes.

While developing #944 we resumed sessions repeatedly (undo history is
persisted per chat), which is exactly the path that triggers this — a
normal user hits it whenever they continue a session whose run state
contains a cyclic schema.

Fix

Try the fast JSON path first; if it throws, fall back to lodash
cloneDeep, which handles cyclic structures. This mirrors what
cloneSessionState already does in the same file.

Safety

  • The normal path is unchanged: when the state has no cycles, behavior is
    identical to before (same JSON round-trip).
  • cloneDeep is already imported and used in this file — no new deps.
  • The fallback only runs in the case that previously crashed.
  • Validated with bun run typecheck (sdk) and by re-running the resume
    flow that originally reproduced the crash.

@codebuff-team

Copy link
Copy Markdown
Contributor

Good diagnosis and a minimal, low-risk fix. The root cause (JSON.stringify choking on cyclic zod lazy schemas embedded in tool blocks) is clearly stated, and the fix is consistent with the existing cloneDeep fallback already used by cloneSessionState in the same file — so this isn't introducing a new pattern, just applying the existing one where it was missed.

A couple of things worth tightening before this lands upstream:

  1. The bare catch {} swallows any error from the JSON round-trip, not just the cyclic-structure case. That's consistent with how cloneSessionState does it, so it's not a regression, but it would be worth checking (or at least commenting) whether other JSON.stringify failure modes (e.g. BigInt, undefined in unexpected places) could silently fall into the cloneDeep path with different semantics than intended.
  2. No test was added. Since this is a fix for a crash with a concrete repro (resume flow with a cyclic run-state), a small regression test constructing a session state with a self-referential/lazy value and asserting applyOverridesToSessionState doesn't throw would make this much easier to port and keep fixed.

Otherwise this is exactly the kind of targeted, well-explained fix that's easy to port by hand — small diff, clear cause, no scope creep.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants