Skip to content

fix(agents): preserve final text after stale incomplete-turn state#80931

Closed
hclsys wants to merge 1 commit into
openclaw:mainfrom
hclsys:fix/incomplete-turn-preserve-final-text-80918
Closed

fix(agents): preserve final text after stale incomplete-turn state#80931
hclsys wants to merge 1 commit into
openclaw:mainfrom
hclsys:fix/incomplete-turn-preserve-final-text-80918

Conversation

@hclsys

@hclsys hclsys commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Fixes #80918

Summary

  • Treat currentAttemptAssistant as the freshest terminal assistant state for incomplete-turn detection.
  • Pass that same terminal assistant into payload construction and final assistant metadata.
  • Preserve the post-update_plan final answer when lastAssistant is still the previous toolUse snapshot.

Real behavior proof

  • Behavior or issue addressed: A run shaped as assistant(toolUse update_plan) -> toolResult(empty) -> assistant(stop, visible final text) must not be classified as incomplete just because lastAssistant still points at the previous toolUse snapshot.
  • Real environment tested: Local OpenClaw source checkout on Node 22.22.0, importing the patched runtime helper directly from src/agents/pi-embedded-runner/run/incomplete-turn.ts with node --conditions=openclaw-source --import tsx. No live WhatsApp/OpenRouter credentials were used.
  • Exact steps or command run after this patch:
    • Runtime probe: node --conditions=openclaw-source --import tsx -e '<imports resolveIncompleteTurnPayloadText from source; stale lastAssistant=toolUse; fresh currentAttemptAssistant=stop; payloadCount=1>'
    • Supplemental regression checks: pnpm test src/agents/pi-embedded-runner/run.incomplete-turn.test.ts -- --reporter=verbose
    • Supplemental adjacent checks: pnpm test src/agents/pi-embedded-runner/run/payloads.test.ts src/cron/isolated-agent/helpers.test.ts -- --reporter=verbose
    • Supplemental project checks: pnpm run check:changed
  • Evidence after fix: Terminal output from the runtime probe against the patched OpenClaw source tree:
$ node --conditions=openclaw-source --import tsx -e '<imports resolveIncompleteTurnPayloadText from source; stale lastAssistant=toolUse; fresh currentAttemptAssistant=stop; payloadCount=1>'
{
  "staleLastAssistantStopReason": "toolUse",
  "currentAttemptAssistantStopReason": "stop",
  "payloadCount": 1,
  "incompleteTurnPayload": null,
  "visibleFinalText": "Final user-visible answer after update_plan."
}

Supplemental local checks also passed: run.incomplete-turn.test.ts 94/94, run/payloads.test.ts 24/24, cron/isolated-agent/helpers.test.ts 22/22, and check:changed exited 0 with 0 oxlint warnings/errors plus 0 runtime value import cycles.

  • Observed result after fix: With stale lastAssistant.stopReason = "toolUse" and fresh currentAttemptAssistant.stopReason = "stop", the patched source returns incompleteTurnPayload: null while preserving the visible final text. That is the non-error branch needed for the issue's post-update_plan final answer.
  • What was not tested: I did not run a live WhatsApp/OpenRouter reproduction. The reporter's production trace covers the live channel symptom; this PR covers the source-level data-loss branch and regression tests.

Risk

  • Low scope: changes only terminal assistant selection in the embedded runner and incomplete-turn helper.
  • Existing #76477 guard remains covered: terminal toolUse with only pre-tool text still surfaces the incomplete-turn error.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. size: S labels May 12, 2026
@clawsweeper

clawsweeper Bot commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR makes the embedded runner prefer currentAttemptAssistant over stale lastAssistant for incomplete-turn detection, payload construction, final assistant metadata, and adds regressions for the post-update_plan final-text case.

Reproducibility: yes. source-level reproduction is high confidence: current main reads stale attempt.lastAssistant in the incomplete-turn classifier and payload path, while the linked issue provides production trace evidence for the same stale toolUse after update_plan shape. I did not run the live WhatsApp channel path during this read-only review.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal output from a runtime probe against the patched source showing the stale lastAssistant scenario now preserves final visible text and avoids the incomplete-turn payload.

Next step before merge
No repair lane is needed because the PR is focused, proof is supplied, and I found no actionable code defect for automation to fix.

Security
Cleared: The diff only changes embedded runner selection logic and tests; it does not touch dependencies, workflows, secrets, install scripts, or other supply-chain surfaces.

Review details

Best possible solution:

Land the narrow terminal-assistant selection fix with its regression tests if required checks stay green, and track any broader observability or recovery work separately.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level reproduction is high confidence: current main reads stale attempt.lastAssistant in the incomplete-turn classifier and payload path, while the linked issue provides production trace evidence for the same stale toolUse after update_plan shape. I did not run the live WhatsApp channel path during this read-only review.

Is this the best way to solve the issue?

Yes, this is a narrow maintainable fix: selecting currentAttemptAssistant ?? lastAssistant at the classifier and payload boundary addresses the stale-state bug without weakening the prior terminal toolUse guard. Broader delivery observability and recovery proposals should remain separate from this fix.

What I checked:

  • Current-main root cause path: On current main, resolveIncompleteTurnPayloadText bases the tool-use terminal guard and incomplete-terminal check on params.attempt.lastAssistant, so a stale toolUse snapshot can still drive the incomplete-turn branch even when currentAttemptAssistant is a completed text turn. (src/agents/pi-embedded-runner/run/incomplete-turn.ts:234, da7f9a62676c)
  • Current-main payload path: On current main, final assistant metadata is resolved from sessionLastAssistant and buildEmbeddedRunPayloads receives attempt.lastAssistant, matching the stale snapshot failure mode described by the linked issue. (src/agents/pi-embedded-runner/run.ts:2473, da7f9a62676c)
  • PR diff scope: The PR introduces terminalAssistant = currentAttemptAssistant ?? lastAssistant in the helper, uses the same selected assistant for runner payload/final metadata construction, and adds helper plus runner regression tests for the stale lastAssistant after update_plan case. (src/agents/pi-embedded-runner/run.ts:2470, 4bc5e2aa53c4)
  • Linked issue and proof: The PR body links the open production report, supplies a terminal runtime probe against the patched source showing incompleteTurnPayload: null with stale lastAssistant=toolUse and fresh currentAttemptAssistant=stop, and reports focused regression plus changed checks passing. (4bc5e2aa53c4)
  • Related search: GitHub search found the linked issue and PR as the central current items for incomplete-turn plus update_plan; other results were broader update_plan churn, planning-only, or older final-text/tool-use work rather than a merged superseding fix.
  • History and routing: Path history points to @amknight for the earlier incomplete tool-use guard, @steipete for recent incomplete-turn/payload work, and @hclsys for prior merged embedded-runner failover work in the same central runner file. (src/agents/pi-embedded-runner/run/incomplete-turn.ts:223)

Likely related people:

  • amknight: GitHub path history shows commit a92e2b1 added the current incomplete tool-use guard for the earlier final-text loss report, which is the behavior this PR narrows for stale snapshots. (role: introduced adjacent behavior; confidence: high; commits: a92e2b13b8b8; files: src/agents/pi-embedded-runner/run/incomplete-turn.ts, src/agents/pi-embedded-runner/run.incomplete-turn.test.ts)
  • steipete: Recent commits on the same incomplete-turn and embedded payload surfaces include canonical reply payload handling and tighter incomplete-turn assertions. (role: recent area contributor; confidence: high; commits: 83a108080de7, 8c320866dc1e, 9e9df8f2c578; files: src/agents/pi-embedded-runner/run.incomplete-turn.test.ts, src/agents/pi-embedded-runner/run/incomplete-turn.ts, src/agents/pi-embedded-runner/run.ts)
  • hclsys: Although this is the PR author, GitHub path history also shows prior merged work by this contributor in the same embedded runner file for failover behavior. (role: recent area contributor; confidence: medium; commits: 398dd6e0b091; files: src/agents/pi-embedded-runner/run.ts)

Remaining risk / open question:

  • I did not inspect a live WhatsApp/OpenRouter end-to-end run in this read-only review; the supplied proof is a terminal runtime probe plus CI/test evidence for the source-level branch.
  • The linked issue includes broader observability and recovery ideas that are outside this PR’s narrow terminal-assistant selection fix.

Codex review notes: model gpt-5.5, reasoning high; reviewed against da7f9a62676c.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 12, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 12, 2026
@hclsys

This comment was marked as low quality.

Sign in to view
@clawsweeper clawsweeper Bot mentioned this pull request May 16, 2026
Closed
This was referenced Jun 17, 2026
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Silent send miss: incomplete-turn classifier discards stopReason=stop final after update_plan

1 participant

@hclsys