Skip to content

fix(agents): surface user-visible error when embedded session is stuck or overflows context (#84536)#84602

Closed
lml2468 wants to merge 1 commit into
openclaw:mainfrom
lml2468:fix/issue-84536
Closed

fix(agents): surface user-visible error when embedded session is stuck or overflows context (#84536)#84602
lml2468 wants to merge 1 commit into
openclaw:mainfrom
lml2468:fix/issue-84536

Conversation

@lml2468

@lml2468 lml2468 commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #84536.

Problem

Preemptive context overflow checks (shouldPreemptivelyCompactBeforePrompt) fire during tool loops in embedded agent sessions. The session ends with embedded_run_agent_end + error, but no error message is delivered to the channel. Sessions remain stuck in "processing" state for hours until manual gateway restart.

Root Causes

Bug 1: Dead-code overflow fallback in agent-runner-execution.ts

The !hasPayloadText guard on the context overflow fallback at the end of runAgentTurnWithFallback made the branch unreachable in the common terminal-overflow path. run.ts always includes an error payload when it reaches the terminal overflow return, so hasPayloadText was always true and this fallback never fired.

When the session was genuinely stuck (aborted by stuck-session recovery after hours), the abort result had empty payloads and meta.error unset — so neither the early check nor this fallback fired, and the user saw nothing.

Fix: Remove the !hasPayloadText guard. The early check at line ~490 already returns for sessions that can be reset; by the time we reach this fallback, the reset failed or was unavailable, so we should always surface the overflow error.

Bug 2: Stuck-session recovery produces no user notification

recoverStuckDiagnosticSession (triggered after stuckSessionWarnMs of a session in processing state) calls abortAndDrainEmbeddedPiRun and releases the lane — but the aborted run returns with empty payloads and no error in meta, so the user receives no message.

Fix: Thread the abort reason ("stuck_recovery") through:

  1. abortAndDrainEmbeddedPiRun → handle.abort(reason) (when reason is provided)
  2. handle.abort(reason) → runAbortController.abort(reason) (abort signal now carries reason)
  3. runAbortController.signal.reason → EmbeddedRunAttemptResult.abortReason
  4. attempt.abortReason === "stuck_recovery" in run.ts → synthesize a user-visible error payload

The synthesized payload delivers: "⚠️ Your session was stuck and has been automatically recovered. Please try again."

Changes

File Change
src/auto-reply/reply/agent-runner-execution.ts Remove !hasPayloadText dead guard; always surface overflow error
src/agents/pi-embedded-runner/run-state.ts EmbeddedPiQueueHandle.abort: (reason?: unknown) => void
src/agents/pi-embedded-runner/runs.ts Pass params.reason to handle.abort(reason) in abortAndDrainEmbeddedPiRun
src/agents/pi-embedded-runner/run/types.ts Add abortReason?: string to EmbeddedRunAttemptResult
src/agents/pi-embedded-runner/run/attempt.ts Read runAbortController.signal.reason and expose as abortReason
src/agents/pi-embedded-runner/run.ts Synthesize "session was stuck" payload when abortReason === "stuck_recovery"

Design Notes

  • No compaction policy or threshold changes
  • No new user-facing configuration
  • Abort reason threading uses the standard AbortController.abort(reason) / signal.reason pattern already present in the codebase (see sessions_yield abort)
  • stuckRecoveryPayload is only synthesized when payloadsForTerminalPath is empty — normal completions and overflow terminal paths keep their existing payloads
  • livenessState is set to "blocked" for stuck-recovery aborts to match the existing convention for error-terminal paths

Acceptance criteria

node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run.overflow-compaction.loop.test.ts
node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts
node scripts/run-vitest.mjs src/agents/pi-embedded-subscribe.handlers.lifecycle.test.ts

Fix commit (f4e723e)

Root cause fix: queueHandle.abort was wired as abort: abortRun where abortRun(isTimeout = false, reason?) takes the timeout flag as first arg. Calling handle.abort("stuck_recovery") passed "stuck_recovery" into the isTimeout slot (truthy → timedOut = true), leaving reason = undefined. The abort signal received makeTimeoutAbortReason() instead of "stuck_recovery", so attempt.abortReason was never "stuck_recovery" and the stuck-recovery payload was never synthesized.

Fix: src/agents/pi-embedded-runner/run/attempt.ts:3314

-  abort: abortRun,
+  abort: (reason?: unknown) => abortRun(false, reason),

This matches the updated EmbeddedPiQueueHandle type (abort: (reason?: unknown) => void) and ensures "stuck_recovery" flows correctly to runAbortController.abort(reason) → signal.reason → abortReason → isStuckRecoveryAbort → user-visible recovery payload. ✓

Tests added: src/agents/pi-embedded-runner/runs.test.ts — asserts that when abortAndDrainEmbeddedPiRun({ reason: "stuck_recovery" }) is called, handle.abort receives "stuck_recovery" as the reason (not isTimeout).

All three acceptance criteria test commands pass.

@X-MerlinLee
fix(agents): surface user-visible error when embedded session is stuc…
facafff 
…k or overflows context

Fixes openclaw#84536.

Two root causes addressed:

1. Dead-code guard in agent-runner-execution.ts
   The hasPayloadText guard on the context overflow fallback made the
   branch unreachable in the common terminal-overflow path, because
   run.ts always includes an error payload when it reaches the terminal
   overflow return. This silently fell through to the success path where
   the payload might still get delivered, but the fallback never fired
   for aborted sessions.
   Fix: remove the guard so the friendly overflow message is always
   surfaced when meta.error is a context overflow error.

2. Stuck-session recovery produces no user notification
   recoverStuckDiagnosticSession aborts the embedded run and releases
   the lane, but the abort result had empty payloads so the user saw
   nothing.

   Fix: thread the abort reason (stuck_recovery) from
   abortAndDrainEmbeddedPiRun through handle.abort(reason) to
   AbortController.abort(reason), expose it as
   EmbeddedRunAttemptResult.abortReason, and in run.ts synthesize a
   user-visible error payload when abortReason is stuck_recovery and
   no other payload was generated.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 4:29 PM ET / 20:29 UTC.

Summary
The PR changes embedded runner abort handling and auto-reply context-overflow fallback logic to surface a user-visible error when an embedded session is aborted by stuck-session recovery or terminal context overflow.

PR surface: Source +56. Total +56 across 6 files.

Reproducibility: yes. at source level, but not as a live current-main reproduction. Current main still has the overflow fallback gate and reason-dropping abort path, while the PR head still loses stuck_recovery before the synthesized payload can fire.

Review metrics: 1 noteworthy metric.

  • Retired runner paths: 5 changed files under src/agents/pi-embedded-runner; current main uses src/agents/embedded-agent-runner. The PR cannot be reviewed as a direct current-main patch until the useful changes are ported to the active runner path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84536
Summary: This PR is a candidate fix for the canonical silent embedded context-overflow and stuck-recovery notification issue; broader delivery and stuck-recovery work overlaps but does not fully replace the unique overflow-delivery fix.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix the active handle so stuck_recovery reaches AbortSignal.reason, and add regression coverage for that exact boundary.
  • Rebase or port the change from retired pi-embedded-runner paths to current embedded-agent-runner paths.
  • [P1] Add redacted real behavior proof that the final recovery or overflow error reaches a real user-visible channel.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists focused Vitest commands, but there is no after-fix live/terminal/channel proof showing the user-visible recovery or overflow message delivered; redacted logs, terminal output, screenshots, or a recording should be added before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A native Telegram recording can show whether the final recovery or overflow error is delivered once instead of leaving the chat silent. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: trigger an embedded agent context-overflow or stuck-recovery path and show the user-visible recovery/error message is delivered once.

Risk before merge

  • [P1] The PR head is conflicting and still edits retired src/agents/pi-embedded-runner paths, so it needs a current-main port before the code can be evaluated as a landing candidate.
  • [P2] The central stuck-recovery notification path is ineffective on the submitted head because handle.abort(params.reason) still calls an abortRun signature that treats the first argument as the timeout flag.
  • [P1] The change affects final user-visible agent recovery messages, but the PR only supplies test-command claims and the Real behavior proof check is failing.

Maintainer options:

  1. Port and prove the current runner fix (recommended)
    Repair the abort-reason contract on current embedded-agent-runner paths, add focused regression coverage, and include real redacted proof that the final user-visible message is delivered.
  2. Pause if the replacement path takes over
    If maintainers choose fix(agent): make stuck recovery phase-safe #93655 or a new current-main replacement as the canonical fix, leave this branch paused until the unique overflow-delivery part is either included there or explicitly dropped.

Next step before merge

  • [P1] The remaining work needs maintainer/contributor handling because the branch is conflicting, the central abort-reason bug is still present, and external real behavior proof cannot be supplied by an automated repair lane.

Security
Cleared: The diff is limited to embedded-run abort/error handling and adds no dependency, workflow, package, permission, secret, or supply-chain surface.

Review findings

  • [P1] Preserve the abort reason before synthesizing recovery text — src/agents/pi-embedded-runner/runs.ts:521
Review details

Best possible solution:

Port the useful fix to current src/agents/embedded-agent-runner paths, preserve stuck-recovery abort reasons through the active-run boundary, coordinate with #93655, and add focused regression tests plus real channel-visible proof before merge.

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

Yes at source level, but not as a live current-main reproduction. Current main still has the overflow fallback gate and reason-dropping abort path, while the PR head still loses stuck_recovery before the synthesized payload can fire.

Is this the best way to solve the issue?

No as written. The fix belongs in this owner boundary, but the submitted branch must be ported to current main and must repair the active-handle abort signature before it can solve the linked bug.

Full review comments:

  • [P1] Preserve the abort reason before synthesizing recovery text — src/agents/pi-embedded-runner/runs.ts:521
    This call does not actually pass "stuck_recovery" into AbortSignal.reason: the active handle is still implemented as abort: abortRun, and abortRun takes the timeout flag as its first parameter. Passing the string here makes it a truthy timeout flag, so attempt.abortReason remains undefined and the new stuck-recovery payload never fires.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 640258d7b31d.

Label changes

Label justifications:

  • P1: The linked bug can leave real agent/channel sessions silently stuck without a user-visible terminal error.
  • merge-risk: 🚨 session-state: The diff changes embedded-run abort and liveness handling, and the submitted head still loses the recovery reason needed to clear the stuck-session path correctly.
  • merge-risk: 🚨 message-delivery: The purpose of the patch is final user-visible recovery/error delivery, and an incorrect merge could continue suppressing that message.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists focused Vitest commands, but there is no after-fix live/terminal/channel proof showing the user-visible recovery or overflow message delivered; redacted logs, terminal output, screenshots, or a recording should be added before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes user-visible agent recovery/error text that can surface in Telegram, so a short Telegram Desktop proof would materially help review.
Evidence reviewed

PR surface:

Source +56. Total +56 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 6 63 7 +56
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 63 7 +56

What I checked:

  • Repository policy read: Read the root and scoped agent runner AGENTS.md files; the review applied the deep source-reading, current-main, session-state, proof, and Telegram-visible proof guidance. (AGENTS.md:1, 640258d7b31d)
  • Live PR state: GitHub reports the PR head as conflicting/DIRTY, with five changed files under retired src/agents/pi-embedded-runner paths and a failing Real behavior proof check. (facafffb0b47)
  • Current main overflow fallback still matches reported gap: Current main still gates the embedded context-overflow fallback on !hasPayloadText, so current main has not already implemented this PR's overflow fallback change. (src/auto-reply/reply/agent-runner-execution.ts:3351, 640258d7b31d)
  • Current main stuck-recovery reason is not forwarded: abortAndDrainEmbeddedAgentRun accepts reason but calls abortEmbeddedAgentRun(params.sessionId) without passing that reason into the active run handle. (src/agents/embedded-agent-runner/runs.ts:660, 640258d7b31d)
  • Current main active handle only preserves restart semantics: The active run abort callback only maps restart to a concrete abort error; stuck_recovery is not a supported reason in current main. (src/agents/embedded-agent-runner/run/attempt.ts:3715, 640258d7b31d)
  • PR-head abort reason contract is still broken: The PR calls handle.abort(params.reason), but the active handle is still abort: abortRun; in that runner, the first argument is the timeout flag, so the string reason is consumed in the wrong parameter and attempt.abortReason remains undefined. (src/agents/pi-embedded-runner/runs.ts:521, facafffb0b47)

Likely related people:

  • keshav55: The preemptive context-overflow detection during tool loops named by the linked issue was added in commit 3aa4199 and lists this handle in commit metadata. (role: introduced related behavior; confidence: high; commits: 3aa4199ef098; files: src/agents/embedded-agent-runner/tool-result-context-guard.ts)
  • jalehman: The preemptive overflow commit and the abort/drain commit both list this handle as reviewer or co-author in commit metadata. (role: reviewer and adjacent contributor; confidence: medium; commits: 3aa4199ef098, 54be30ef89f5; files: src/agents/embedded-agent-runner/tool-result-context-guard.ts, src/agents/embedded-agent-runner/runs.ts)
  • cgdusek: Commit 54be30e changed the embedded-run abort/drain area used by diagnostic stuck-session recovery. (role: abort/drain contributor; confidence: medium; commits: 54be30ef89f5; files: src/agents/embedded-agent-runner/runs.ts)
  • Takhoffman: Commits 3e2a05f and 66daafc refined the reserve-based and cause-aware precheck overflow routing around the same failure family. (role: adjacent precheck contributor; confidence: medium; commits: 3e2a05f4251f, 66daafccae09; files: src/agents/embedded-agent-runner/run/preemptive-compaction.ts, src/agents/embedded-agent-runner/run/attempt.ts)
  • steipete: Adjacent overflow recovery and user-visible fallback commits touched the same overflow-delivery family around this report. (role: adjacent recovery contributor; confidence: medium; commits: a42ee69ad4a7, eb73e87f18d1; files: src/agents/embedded-agent-runner/tool-result-context-guard.ts, src/auto-reply/reply/agent-runner-execution.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper clawsweeper Bot mentioned this pull request May 22, 2026
Closed
@lml2468

lml2468 commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Fixed the P1 abort reason contract bug: queueHandle.abort was wired directly to abortRun(isTimeout, reason?), causing handle.abort("stuck_recovery") to set isTimeout=true with reason=undefined. Changed to (reason?) => abortRun(false, reason) so the reason flows through to AbortSignal.reason correctly.

Also added a regression test in runs.test.ts covering reason propagation through the abort chain.

@clawsweeper

clawsweeper Bot commented May 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@osolmaz osolmaz mentioned this pull request May 28, 2026
Open
@liaoandi liaoandi mentioned this pull request May 29, 2026
Closed
This was referenced Jun 15, 2026
Closed
Open
Closed
@galiniliev galiniliev self-assigned this Jun 23, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 23, 2026

Copy link
Copy Markdown

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed stale Marked as stale due to inactivity labels Jun 23, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 24, 2026

Copy link
Copy Markdown

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 24, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Open
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 8, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

@openclaw-barnacle openclaw-barnacle Bot closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@galiniliev galiniliev

Labels

agents Agent runtime and tooling mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Preemptive context overflow silently kills embedded sessions without notifying user

3 participants

@lml2468 @galiniliev @X-MerlinLee