fix(resume): preserve checkpoints after Ctrl-C

[Gongyl01]

Summary

Follow-up to #845.

#845 made the parent run manifest authoritative for checkpoint reuse. However, ocr review still used the default SIGINT behavior, so Ctrl-C terminated the process before Agent.Run could finalize the manifest and write session_end. The completed review_item_done records remained on disk, but --resume rejected the session because no manifest backed them.

This PR restores Ctrl-C resume without weakening the trusted-resume contract.

Changes

• Install a SIGINT-aware context in ocr review and propagate it through resume validation, MCP initialization, preview, and Agent.Run.
• Make file dispatch cancellation-aware so it stops waiting for new concurrency slots after Ctrl-C.
• Wait for already-dispatched files and asynchronous comment work to finish cancellation before finalizing the run.
• Record the run as cancelled, then use the existing manifest finalization path to write session_end.
• Keep completed files reusable; active and undispatched files are recorded as cancelled and reviewed again on the next --resume.
• Add an integration test that cancels a three-file review, reloads the persisted session, and verifies that only the completed checkpoint is reusable.
• Update the CLI reference in all supported documentation languages.

Non-graceful termination still does not produce a resumable manifest.

Type of Change

• Bug fix
• Documentation update

Verification

• make check
• make test
• make coverage — 91.5%

[github-actions]

✅ OpenCodeReview: Review partially complete: 0 finding(s); 1 of 2 selected item(s) failed.

[wu21-web]

Thank you for your contribution, I really appreciate it.

[wu21-web]

Suggested change

	select {
	case sem <- struct{}{}: // acquire semaphore
	case <-ctx.Done():
	break dispatchLoop
	}
	dispatched++
	wg.Add(1)
	select {
	case sem <- struct{}{}: // acquire semaphore
	case <-ctx.Done():
	break dispatchLoop
	}
	if ctx.Err() != nil {
	<-sem // release the slot acquired concurrently with cancellation
	break dispatchLoop
	}
	dispatched++
	wg.Add(1)

[wu21-web]

Suggested change

	ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt)
	defer stop()
	return executeReviewContext(ctx, reviewOpts)
	ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt)
	defer stop()
	go func() {
	<-ctx.Done()
	stop()
	}()
	return executeReviewContext(ctx, reviewOpts)

[Gongyl01]

Thanks for the suggestion. I’d prefer not to restore default SIGINT handling before graceful cancellation has written session_end and the manifest, because a second Ctrl-C could otherwise leave the session non-resumable. Forced termination would be better handled separately with an explicit warning.

[lizhengfeng101]

Nice work — the signal handling and dispatch-loop changes are clean, and the integration test is well-designed. A few suggestions:

---

1. Comment on the DeadlineExceeded vs Canceled asymmetry in recordContextFailure

DeadlineExceeded uses SetPendingFailureCause while Canceled uses SetRunFailure. The distinction makes sense (user-initiated cancellation is a run-level event; a deadline is item-attributable), but it's non-obvious to a reader who'd expect symmetric handling. A short comment explaining the "why" would help future maintainers:

func (a *Agent) recordContextFailure(err error) {
	if b := a.session.Manifest(); b != nil {
		var setErr error
		if errors.Is(err, context.DeadlineExceeded) {
			// Deadline is a pending cause, not a run failure: individual items
			// may have completed before the deadline, so coverage determines
			// the terminal state.
			setErr = b.SetPendingFailureCause(session.FailureTimeout, "review deadline exceeded")
		} else {
			// Ctrl-C is an explicit user action — record as a run-level failure.
			setErr = b.SetRunFailure(session.RunFailureCancelled, "review was cancelled")
		}
		...
	}
}

---

2. Consider moving executeReview/runPreview wrappers to test scope

After this PR, both wrappers are only called from tests (compat_test.go, review_helpers_test.go). They could live in an export_test.go to make it explicit they're test-only entry points. Not a blocker — just a tidiness note.

---

3. Missing edge case: cancellation before any file is dispatched

The test covers the "one completed, one in-flight, one pending" scenario nicely. One case that isn't exercised: Ctrl-C fires while the very first semaphore acquire is pending (e.g., concurrency slot held by a preceding long-running resume-validation or the sem starts full for some other reason), so dispatched == 0 when ctx.Err() != nil. The current code handles it correctly, but a small test would lock in that guarantee — especially since the dispatched == 0 check was reordered relative to the ctx-error check in this PR.

---

Overall this is solid and ready to merge with or without the above. Thanks for the thorough doc updates across all four languages.

[lizhengfeng101]

LGTM