Skip to content

fix(agent): clear retryable error when agent ends its turn without JSON - #813

Open
Lifferado wants to merge 1 commit into
kunchenguid:mainfrom
Lifferado:fm/nm-fix-c2
Open

fix(agent): clear retryable error when agent ends its turn without JSON#813
Lifferado wants to merge 1 commit into
kunchenguid:mainfrom
Lifferado:fm/nm-fix-c2

Conversation

@Lifferado

Copy link
Copy Markdown

Fixes #811

Root cause

When a pipeline agent ends its final turn with plain prose - an early narrative stop, or a turn aborted mid-task by provider stream errors (usage caps, content filters, 503s) - opencode reports no info.error = StructuredOutputError, so the #375 path does not trigger. The text fallback in finalizeTextResult/parseStructuredTextOutput then feeds the narration to encoding/json, and every candidate (whole text, fences, last bare object) fails on the first byte. The step dies with opencode output parse: invalid character 'L' looking for beginning of value (output snippet: "Let me check the remaining ...") after minutes of real work, with no retry: prose is neither transient nor a StructuredOutputError.

Fix

  1. Dedicated error (internal/agent/agent.go): when no candidate parses AND the text shows no attempt at JSON at all - it neither starts with { nor contains a ```json fence - parseStructuredTextOutput returns the new sentinel `errTurnEndedWithoutJSON` ("agent ended its turn without the required JSON result"). The step error now reads `opencode output parse: agent ended its turn without the required JSON result (output snippet: "...")`. Malformed output that DOES attempt JSON (leading `{`, fenced JSON, embedded bare object) keeps today's precise decoder/validation error unchanged.
  2. Retryable with reminder (internal/agent/retry.go): classifyTransient treats the sentinel as retryable (identity check via errors.Is, so the same words echoed in provider output never retry), bounded by the existing per-adapter retry budget. On such a retry, runWithRetry appends a one-time prompt reminder that the final assistant message must be exactly one bare JSON object; adapters that resume a durable session therefore re-invoke that same session with the reminder. runOnce now receives the effective RunOpts so every adapter gets this uniformly.
  3. Regression tests: prose through finalizeTextResult asserts the sentinel and that no decoder error leaks; attempted-JSON cases assert old behavior is preserved; classifier tests assert retryability and identity semantics; retry-loop tests assert the untouched first prompt, the appended reminder, exactly-once appending, bounded attempts, and recovery when the next attempt succeeds.

The telemetry failure bucket is unaffected: the wrapped message still contains "output parse", which classifyInvocationFailure already maps to "parse".

When a pipeline agent ends its final turn with plain prose - an early
narrative stop or a turn aborted mid-task by provider stream errors -
every structured-output candidate failed on the first byte and the raw
encoding/json decoder error ("invalid character 'L' looking for
beginning of value") leaked as the step error, misdirecting diagnosis
toward format problems and ending the run with no retry (kunchenguid#811).

parseStructuredTextOutput now returns a dedicated sentinel,
errTurnEndedWithoutJSON, when no candidate parsed AND the text shows no
attempt at JSON at all (no leading '{', no ```json fence). Malformed
output that does attempt JSON keeps today's precise decoder/validation
error. classifyTransient treats the sentinel as retryable, and
runWithRetry re-invokes the next attempt (same session for adapters
that resume one) with a one-time prompt reminder that the final message
must be exactly the bare JSON object.

Regressions: TestFinalizeTextResult_ProseFinalTurnReturnsDedicatedError,
TestFinalizeTextResult_AttemptedJSONKeepsRawParseError,
TestClassifyTransient_TurnEndedWithoutJSONIsRetryable,
TestRunWithRetry_TurnEndedWithoutJSONRetriesWithReminder,
TestRunWithRetry_TurnEndedWithoutJSONRecoversWhenNextAttemptSucceeds.

Fixes kunchenguid#811
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The malformed embedded-JSON classification should be corrected before merging so truncated JSON attempts do not trigger misleading and unnecessary retries.

A response that begins with narration and then starts an incomplete JSON object bypasses the new attempt detector, replacing its precise decoder error with the retryable missing-JSON sentinel.

Files Needing Attention: internal/agent/agent.go, internal/agent/agent_test.go

Reviews (1): Last reviewed commit: "fix(agent): clear retryable error when a..." | Re-trigger Greptile

Comment thread internal/agent/agent.go
Comment on lines +384 to +386
if strings.HasPrefix(strings.TrimLeft(text, " \t\r\n"), "{") {
return true
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Embedded JSON attempts are missed

When narration precedes a truncated JSON object such as Let me finish... {"findings": [, this prefix-only check classifies the response as having made no JSON attempt, causing unnecessary retries and replacing the precise malformed-JSON error with the misleading missing-result sentinel.

Suggested change
if strings.HasPrefix(strings.TrimLeft(text, " \t\r\n"), "{") {
return true
}
if strings.Contains(text, "{") {
return true
}

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: inspected the live diff vs main at 6f1c65f9f2a7.

Classification: corrective. Fixes #811: a prose-only final turn no longer leaks invalid character 'L' looking for beginning of value. parseStructuredTextOutput returns errTurnEndedWithoutJSON when no candidate parses and the text never attempted JSON; attempted-JSON failures keep the precise decoder error. classifyTransient retries that sentinel by identity (not string match), and runWithRetry appends a one-time JSON reminder. Reviewers stay independent; this is mechanical retry of a missing structured result.

VISION: R1 pass (no core-step change); R2 n/a; R3 pass (judgment stays human; retry is mechanical); R4 n/a; R5 pass (clearer attributed error + recorded retry); R6 pass (all adapters get the same reminder path); R7 pass (field incident #811).

Required CI: Generated files must not be hand-edited SUCCESS, check SUCCESS, e2e SUCCESS, test (windows-core) SUCCESS; test (ubuntu-latest), test (macos-latest), test (windows-git) still running. PR must be raised via no-mistakes FAILED on this head: the body has no git push no-mistakes signature and no no-mistakes-pipeline-attestation:v1 bound to 6f1c65f9. Greptile is not required (FAILURE). MERGEABLE / UNSTABLE.

Not auto-merging while the no-mistakes attestation is missing. This is waiting on the author to re-run git push no-mistakes so the PR body attestation binds to the current head. It is not waiting on the captain.

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

Labels

None yet

Projects

None yet

2 participants