cr review aborts whole run and loses diagnostics when one reviewer returns invalid structured output

[rianjs]

Summary

cr review currently aborts the entire multi-reviewer run when a single reviewer returns structured output that fails schema decoding, and the failed run does not persist enough diagnostic state to identify the offending reviewer or recover successful reviewer work.

Observed

While dogfooding PR #359 on commit 70ee83c17c3980a6e1efb24a7a681bcdf54c4c2a, this command failed after the built-in retry:

/tmp/cr-thread-lifecycle review https://github.com/open-cli-collective/codereview-cli/pull/359 \
  --dry-run --max-agents 5 --allow-self-review --json --verbose

Failure:

structured output invalid after retry: first: json: unknown field "line"; second: json: unknown field "line"

The run selected/logged four reviewers:

• structure:repo-health
• go:implementation-tests
• policies:conventions
• documentation:docs

The ledger run was marked failed, but no sessions or findings were persisted for the failed run. The artifact directory retained only wrapper logs with short Claude background job IDs, and those jobs were no longer available via claude logs, so the raw malformed JSON and offending reviewer could not be recovered.

Example failed run ID:

a53df086-ab7d-4454-b4f6-dc21bdccd5f0

Why this is a bug

Rejecting malformed structured output is correct. The problematic behavior is that one malformed reviewer response:

1. aborts the whole review run, including successful sibling reviewers, and
2. leaves insufficient durable diagnostics to identify the reviewer, inspect the raw invalid output, or resume/recover from the successful work.

For parallel reviewer execution, a schema failure should be isolated to the reviewer/workstream that produced it, or at minimum reported with enough context to debug without relying on ephemeral provider CLI logs.

Desired behavior

• Include reviewer/workstream ID in the structured-output validation error.
• Persist failed reviewer session metadata and validation error details before aborting.
• Persist or artifact the raw invalid structured output, with normal secret-safety constraints.
• Preserve successful reviewer results where possible instead of discarding the whole run.
• Consider allowing the run to complete with a failed-reviewer diagnostic finding/status when one reviewer fails schema validation.

Related symptoms

This is also a debuggability/recovery gap: failed structured-output runs currently depend on ephemeral Claude background logs for raw response inspection, but those logs may be gone by the time the operator investigates.

[rianjs]

Implementation plan is converged and now reflected in the branch:

• Treat selection, each reviewer, and rollup as durable LLM tasks with per-task metadata under llm-tasks/.
• Persist validated outputs, failed validation attempts, provider session IDs, and ledger session rows at each task boundary.
• Isolate reviewer structured-output failures as failed_isolated so sibling reviewers and rollup can continue with diagnostics.
• Classify blocking LLM task failures as incomplete once a run exists, so the existing resume gate can pick them up.
• On resume, load completed selection/reviewer tasks from artifacts and rerun only the first incomplete/failed downstream task, including resuming from the prior provider session ID when available.
• Surface isolated reviewer diagnostics in the rollup and force the review outcome to at least comment when a reviewer failed.

Verification so far:

• rtk go test ./...
• rtk make lint
• rtk make build