Skip to content

fix: recover from truncated resume session - #1317

Open
hyphen0009 wants to merge 1 commit into
alibaba:mainfrom
hyphen0009:fix/resume-session-truncated-json
Open

hyphen0009 wants to merge 1 commit into
alibaba:mainfrom
hyphen0009:fix/resume-session-truncated-json

Conversation

@hyphen0009

Copy link
Copy Markdown

Description

Fixes an issue where ocr scan --resume fails with unexpected end of JSON input after a scan is interrupted with Ctrl+C.

The session uses buffered JSONL persistence, and an interrupted write can leave a truncated final record without a trailing newline. Previously, LoadResumeState treated this incomplete tail record as a fatal parsing error, preventing the scan from resuming.

This change allows resume loading to safely recover from an incomplete final record while keeping malformed records in the middle of the session file as errors.

The scan command also now handles the first Ctrl+C gracefully so checkpoints can be persisted and the session can be closed cleanly.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change
  • Refactoring
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

Added regression tests covering:

  • Truncated final JSONL records
  • Fragment-only session files
  • Valid newline-less final records
  • Mid-file corrupted records
  • Review-session recovery
  • Full abort → resume flow
  • Concurrent checkpoint writes

Verification performed by the development agent:

  • Session tests: PASS
  • Full command test suite: PASS
  • go build ./...: PASS
  • go vet: PASS
  • gofmt: clean

AI Assistance Disclosure

AI assistance was used during development to investigate and implement the fix and to help create the regression tests.

@CLAassistant

CLAassistant commented Sep 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 2 issue(s) in this PR.

  • ✅ Successfully posted inline: 2 comment(s)

//
// A fragment that does parse is folded in as a courtesy to hand-written or
// externally produced files that omit the final newline.
func (s *ResumeState) applyTrailingFragment(sessionID string, fragment []byte) error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainability · low
The sessionID parameter is accepted but never used in this function. The function accesses the session ID through s.SessionID (via applyResumeLine) instead. Either remove the parameter or use it where needed (e.g., for error formatting). Keeping an unused parameter is confusing and will trigger linter warnings.

Suggestion:

Suggested change
func (s *ResumeState) applyTrailingFragment(sessionID string, fragment []byte) error {
func (s *ResumeState) applyTrailingFragment(fragment []byte) error {

Comment on lines +225 to +228
const maxFragment = 256
if len(fragment) > maxFragment {
fragment = fragment[:maxFragment]
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainability · low
When the trailing fragment exceeds 256 bytes, it is silently truncated before being stored in RecoveredFragment. A user inspecting the fragment during diagnosis may not realize it was cut, leading to misleading forensic analysis. Consider appending a truncation indicator (e.g., "...") or storing the original length so the warning output makes the truncation obvious.

Suggestion:

Suggested change
const maxFragment = 256
if len(fragment) > maxFragment {
fragment = fragment[:maxFragment]
}
const maxFragment = 256
if len(fragment) > maxFragment {
fragment = append(fragment[:maxFragment], []byte("...(truncated)")...)
}

@lizhengfeng101

Copy link
Copy Markdown
Contributor

Thanks for the fix! Heads-up: the core of this — tolerating a truncated final JSONL record in loadResumeState — was just landed by #1312 (merged), which fixes the same unexpected end of JSON input on --resume. Both branch from the same resume.go, so this will now conflict with main.

Could you rebase onto main and drop the parts already covered by #1312? A couple of things here still look like net-new value worth keeping:

So: rebase, drop the duplicated tail-truncation handling, and keep the Ctrl+C work (and the newline-less-record recovery if you want it) layered on top. Happy to re-review once it's rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants