Skip to content

fix: skip timeout-triggered compaction when run was aborted by user#82085

Open
lykeion-dev wants to merge 1 commit into
openclaw:mainfrom
lykeion-dev:fix/skip-timeout-compaction-on-abort-5.9
Open

fix: skip timeout-triggered compaction when run was aborted by user#82085
lykeion-dev wants to merge 1 commit into
openclaw:mainfrom
lykeion-dev:fix/skip-timeout-compaction-on-abort-5.9

Conversation

@lykeion-dev

@lykeion-dev lykeion-dev commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Problem: When a user presses the Stop button to abort a run, the timeout-triggered compaction still fires if context usage exceeds 65%. This causes unnecessary compaction at an inappropriate time.
  • Why it matters: User-initiated aborts should not trigger compaction. The user explicitly chose to stop, so compacting and retrying is wrong behavior.
  • What changed: Added !externalAbort guard to both timeout-triggered compaction check and timedOutDuringPrompt evaluation in runEmbeddedPiAgent. externalAbort is set only for user-initiated stops (not internal timeouts), correctly distinguishing user aborts from provider timeouts.
  • What did NOT change: Timeout-triggered compaction still works normally for actual provider timeouts. Only user-initiated aborts are excluded.

Change Type

  • Bug fix

Scope

  • Gateway / orchestration

Linked Issue/PR

  • This PR fixes a bug or regression

Real behavior proof

  • Behavior or issue addressed: Pressing Stop during a long LLM response with high context usage triggers timeout compaction instead of cleanly aborting.
  • Real environment tested: OpenClaw gateway with high-context sessions, user-initiated abort via Stop button.
  • Exact steps: Start a session that builds high context (>65%), press Stop button during LLM response timeout.
  • Evidence after fix: Compaction is skipped when externalAbort=true. Error payload returned without compaction.
  • Observed result after fix: Clean abort without unnecessary compaction.
  • What was not tested: Internal provider timeout paths (unchanged, covered by existing tests).
  • Unit test added: run.timeout-triggered-compaction.test.ts — "does not attempt compaction when user aborted (externalAbort=true)"

Root Cause

  • Root cause: The timeout-triggered compaction guard checked timedOut, timedOutDuringCompaction, and timedOutDuringToolExecution but did not check externalAbort. User-initiated aborts set both aborted and externalAbort, but the compaction code only looked at timeout-related flags.
  • Missing detection: No test covered the externalAbort=true + timedOut=true combination for compaction.
  • Contributing context: externalAbort was introduced to distinguish user stops from internal timeouts, but the compaction path was not updated.

Regression Test Plan

  • Unit test
  • Target test: run.timeout-triggered-compaction.test.ts — "does not attempt compaction when user aborted (externalAbort=true)"
  • Scenario: timedOut=true, aborted=true, externalAbort=true → compaction should NOT fire.
  • Why this is the smallest reliable guardrail: Direct unit test of the exact guard condition.

User-visible / Behavior Changes

  • Pressing Stop during a timeout with high context no longer triggers compaction. The run aborts cleanly.

Security Impact

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

  • Risk: A legitimate timeout during user abort might not get compacted on retry.
    • Mitigation: User abort means no retry. The run ends. Compaction is unnecessary.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 15, 2026
@clawsweeper

clawsweeper Bot commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 8:23 PM ET / July 1, 2026, 00:23 UTC.

Summary
The PR adds externalAbort guards to timeout-triggered compaction and timeout prompt handling in the old PI embedded runner, plus a focused unit test and a changelog entry.

PR surface: Source 0, Tests +21, Docs +1. Total +22 across 3 files.

Reproducibility: yes. source-reproducible: current main's timeout-compaction predicate ignores externalAbort, while the attempt runner can mark active-run cancellation as externalAbort. I did not run a live high-context Stop-button scenario in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Retired runner path targeted: 2 code/test files under src/agents/pi-embedded-runner. Current main uses src/agents/embedded-agent-runner, so these hunks must be ported before they affect runtime behavior.
  • Release-owned changelog touched: 1 CHANGELOG.md entry added. OpenClaw release generation owns changelog edits for normal PRs, so release-note context should stay in PR metadata instead.

Root-cause cluster
Relationship: canonical
Canonical: #82085
Summary: This PR is the current open iteration of the user-aborted timeout-compaction guard; older same-author attempts are closed, while related abort/compaction work covers adjacent paths.

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: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • Port the change and test to src/agents/embedded-agent-runner on current main.
  • [P2] Add redacted real behavior proof showing Stop during a high-context timeout skips compaction.
  • Confirm timeout error feedback still appears when appropriate.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body describes real setup testing but provides no inspectable after-fix terminal output, logs, screenshot, recording, or linked artifact; contributor action is still needed, with private details redacted. 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.

Risk before merge

  • [P1] The PR is currently conflicting and edits the retired src/agents/pi-embedded-runner path, so it cannot land as-is or affect current runtime behavior.
  • [P2] The timedOutDuringPrompt hunk can suppress explicit timeout feedback for stopped-timeout runs; the safer fix is to guard only the compaction attempt predicate unless separate proof supports changing feedback synthesis.
  • [P1] The PR body describes real setup testing, but provides no inspectable terminal output, logs, screenshot, recording, or linked artifact showing the after-fix behavior.
  • [P2] Related abort/fallback work exists, especially fix(agents): distinguish terminal aborts from retryable failures (#60388) #62682, but it is adjacent rather than a safe replacement for this timeout-compaction guard.

Maintainer options:

  1. Port only the compaction guard (recommended)
    Rebase onto current main, apply !externalAbort to the active timeout-compaction predicate, keep timeout feedback logic unchanged, and add focused active-path coverage.
  2. Wait for contributor proof
    Keep the PR blocked until the contributor adds redacted logs, terminal output, a recording, or another inspectable artifact showing Stop during a high-context timeout skips compaction.
  3. Replace the stale branch
    If the contributor does not refresh the conflicting branch, maintainers can open a narrow replacement PR and close this one only after the replacement exists.

Next step before merge

  • [P1] The PR needs contributor or maintainer refresh plus real behavior proof; ClawSweeper should not dispatch a repair marker while the external-contributor proof gate remains unsatisfied.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes agent runner control flow, a focused test, and release-note text only.

Review findings

  • [P1] Port the guard to the active embedded runner — src/agents/pi-embedded-runner/run.ts:1688
  • [P2] Keep timeout feedback independent of external abort — src/agents/pi-embedded-runner/run.ts:2639
  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:31
Review details

Best possible solution:

Port only the timeout-compaction attempt guard to the active embedded runner, keep timeout payload synthesis intact unless separately proven, remove the release-owned changelog edit, add current-path regression coverage, and require redacted real behavior proof before merge.

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

Yes, source-reproducible: current main's timeout-compaction predicate ignores externalAbort, while the attempt runner can mark active-run cancellation as externalAbort. I did not run a live high-context Stop-button scenario in this read-only review.

Is this the best way to solve the issue?

No. externalAbort is the right discriminator for the timeout-compaction attempt gate, but this branch targets the retired runner path and also changes timeout-feedback synthesis, so it is not the best fix as submitted.

Full review comments:

  • [P1] Port the guard to the active embedded runner — src/agents/pi-embedded-runner/run.ts:1688
    This hunk changes src/agents/pi-embedded-runner/run.ts, but current main has no pi-embedded-runner tree and the active timeout-compaction predicate is in src/agents/embedded-agent-runner/run.ts. Rebase and apply the guard/test to the active path; otherwise this branch cannot land or fix current runtime behavior.
    Confidence: 0.95
  • [P2] Keep timeout feedback independent of external abort — src/agents/pi-embedded-runner/run.ts:2639
    Adding !externalAbort to timedOutDuringPrompt makes that flag false for the stopped-timeout case, but the same flag drives explicit timeout error payload synthesis. Keep the external-abort guard on the compaction attempt gate and leave timeout feedback unchanged unless separately proven.
    Confidence: 0.87
  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:31
    OpenClaw release generation owns CHANGELOG.md for normal PRs. Keep the release-note context in the PR body or squash message and drop this file change before merge.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5e572dcf781a.

Label changes

Label justifications:

  • P2: This is a normal-priority agent runtime bug fix with limited blast radius but real Stop-button timeout-compaction impact.
  • merge-risk: 🚨 message-delivery: The proposed timeout-feedback predicate change can alter or suppress explicit timeout feedback sent back to users after stopped timeout runs.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • 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 describes real setup testing but provides no inspectable after-fix terminal output, logs, screenshot, recording, or linked artifact; contributor action is still needed, with private details redacted. 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.
Evidence reviewed

PR surface:

Source 0, Tests +21, Docs +1. Total +22 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 2 0
Tests 1 21 0 +21
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 24 2 +22

What I checked:

  • Repository policy read: Root and scoped agent runner policies were read and applied; they require whole-surface PR review, current-main behavior checks, real behavior proof for user-visible fixes, and no normal PR edits to release-owned changelog content. (AGENTS.md:1, 5e572dcf781a)
  • Live PR state: GitHub reports the PR as open, non-draft, maintainer-editable, and mergeable=CONFLICTING; its diff still targets src/agents/pi-embedded-runner/*. (8b85130cba63)
  • Active runner path: Current main has no src/agents/pi-embedded-runner directory; the active timeout-compaction code and tests live under src/agents/embedded-agent-runner. (src/agents/embedded-agent-runner/run.ts:2475, 5e572dcf781a)
  • Current timeout compaction predicate: Current main still attempts timeout-triggered compaction on timedOut without checking externalAbort, so the useful guard has not already landed on the active path. (src/agents/embedded-agent-runner/run.ts:2475, 5e572dcf781a)
  • Timeout feedback dependency: timedOutDuringPrompt drives explicit timeout payload synthesis; adding !externalAbort there can suppress user-visible timeout feedback separately from skipping compaction. (src/agents/embedded-agent-runner/run.ts:3625, 5e572dcf781a)
  • External abort source: The active attempt runner can set externalAbort = true for active-run cancellation and also sets timeout state when the run timeout path fires, making the requested discriminator relevant to the active code path. (src/agents/embedded-agent-runner/run/attempt.ts:3716, 5e572dcf781a)

Likely related people:

  • joeykrug: Authored the merged timeout-triggered compaction PR and several commits in the original runner/test surface. (role: introduced timeout-compaction behavior; confidence: high; commits: d81593c6e256, c02a0c1ea4f2, 117b5652cad3; files: src/agents/embedded-agent-runner/run.ts, src/agents/embedded-agent-runner/run.timeout-triggered-compaction.test.ts)
  • jalehman: Co-authored/reviewed the introducing timeout-compaction merge and authored follow-up commits aligning the compaction runtime context and recovery recording. (role: reviewer and timeout-compaction refactor contributor; confidence: high; commits: d81593c6e256, 1da1f685fe5a, 192e14aeb289; files: src/agents/embedded-agent-runner/run.ts, src/agents/embedded-agent-runner/run.timeout-triggered-compaction.test.ts)
  • doedewaldt: Authored the merged PR that made prompt timeout payloads explicit, which is the behavior the second hunk can suppress. (role: timeout-feedback contributor; confidence: medium; commits: c7623f706f38, bab9f3ddfe04; files: src/agents/embedded-agent-runner/run.ts, src/agents/embedded-agent-runner/run.timeout-triggered-compaction.test.ts)
  • obviyus: Authored and landed the merged work distinguishing internal aborts from explicit external/user cancellations in the embedded runner. (role: external-abort semantics contributor; confidence: high; commits: f454d6202f1a, 54d2e7458502; files: src/agents/embedded-agent-runner/run/attempt.ts, src/agents/embedded-agent-runner/run/incomplete-turn.ts, src/agents/embedded-agent-runner/run/attempt-trajectory-status.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.

@lykeion-dev
lykeion-dev force-pushed the fix/skip-timeout-compaction-on-abort-5.9 branch 3 times, most recently from d0f275c to edac0c9 Compare May 15, 2026 18:37
@lykeion-dev
lykeion-dev marked this pull request as ready for review May 15, 2026 18:58
@lykeion-dev
lykeion-dev force-pushed the fix/skip-timeout-compaction-on-abort-5.9 branch from edac0c9 to 862dda2 Compare May 15, 2026 19:01
@lykeion-dev
lykeion-dev force-pushed the fix/skip-timeout-compaction-on-abort-5.9 branch from 862dda2 to 8b85130 Compare May 17, 2026 18:20
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels May 17, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 2, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 2, 2026
@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. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels Jun 2, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 15, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. 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. and removed 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. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 15, 2026
This was referenced Jun 19, 2026
Closed
Merged
@clawsweeper clawsweeper Bot removed 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. labels Jun 19, 2026
@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. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 19, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 24, 2026
Merged
3 tasks
@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@lykeion-dev thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

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 merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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.

1 participant

@lykeion-dev