Repository navigation
fix: report failed cloud tools as errors - #139758
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 2:00 AM ET / 06:00 UTC. ClawSweeper reviewWhat this changesThe PR classifies structured tool failures before completion events and transcript storage while preserving explicit plugin recovery. Merge readiness✅ Ready for maintainer review This remains a useful, focused fix: current main still misses structured failures at the session outcome boundary. No introduced correctness or security blocker was found, and the maintainer-authored PR is protected from cleanup closure. Priority: P2 Review scores
Verification
How this fits togetherOpenClaw’s session lifecycle receives tool results and plugin overrides, then produces completion events, saved transcripts, and the next model request. This change makes those consumers receive consistent error metadata. flowchart TD
A[Tool execution or preflight] --> B[Structured result]
B --> C[Session error classification]
C --> D{Execution started?}
D -->|Yes| E[Plugin overrides and recovery]
D -->|No| F[Final outcome]
E --> F
F --> G[Completion events]
F --> H[Transcript and model context]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +6, Tests +152. Total +158 across 3 files. View PR surface stats
Review metrics
Technical reviewBest possible solution: Keep outcome classification at the shared session lifecycle, with explicit middleware recovery authoritative and one finalized flag feeding events, persistence, and model context. Do we have a high-confidence way to reproduce the issue? Yes, from source: an adapted tool exception becomes a structured error result that current main can finalize with isError=false. No runtime reproduction was executed during this read-only review. Is this the best way to solve the issue? Yes. Reusing the existing classifier before middleware and at the pre-execution outcome boundary repairs the producer while retaining explicit recovery and completed-command semantics. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against d1dce5857a3a. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
steipete
marked this pull request as ready for review
September 6, 2026 05:02
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
* fix: report failed cloud tools as errors * test: verify tool outcomes through typed transcript reload * refactor: keep tool outcome proof focused and lint clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Fixes an issue where users running cloud computer tools would see failed operations recorded as successful. A live macOS run returned a screenshot error while its saved tool result said
isError: falseand its trajectory saidsuccess: true. That also gave the next model request incorrect outcome metadata.Why This Change Was Made
Classify structured tool failures at the shared session lifecycle, before completion events and transcript persistence. Executed results are classified before plugin middleware; each middleware result preserves an explicit error override, including intentional recovery. Pre-execution failures use the existing terminal hook because they never enter execution middleware.
This keeps one finalized outcome for local and remote embedded sessions. It adds no protocol, configuration, schema, timeout, retry, or downstream display workaround. The underlying locked/asleep macOS display is a separate readiness issue; this PR does not fix display capture or bypass screen locking.
User Impact
Failed tools are reported truthfully to users and models. Successful commands, including completed commands with nonzero exit codes, retain their existing semantics. Plugins can explicitly recover from errors without discarding diagnostic details. Optional file absence remains an intentional successful query result.
Evidence
Final head:
6f45aeec60f9c74dfb6734cc3350bd96a2dba653.Real OpenAI cloud-agent verification on the retained macOS host used a fresh worker built from this exact revision. Each result was matched by tool call ID in both live trajectory and committed transcript:
Connecting WebVNC in view-only mode alone did not restore native screenshot readiness: that additional failed capture is retained as an error. A single normal wake click allowed capture. The positive image shows the normal OS lock screen, not unlocked app control; no OS login, TCC reset, sleep-policy change, or security bypass was used for this final verification. The task-only worker is reclaimed and its session archived; the retained host remains available.
The same mismatch was observed before the fix. Regression tests drive actual adapted tools through a real session loop and reopen its SQLite transcript: returned errors, execution and preparation throws, missing tools, successful results, completed nonzero commands, explicit recovery, and middleware-added failure, with and without plugin handlers. The original source fails both variants. The explicit-recovery and middleware-combination defects found during review were separately reproduced and fixed.
9c1ba9ec5a4b231ea290afe4829d0c1980b7c64f(760.603 seconds). Environment run. The refresh preserves the identical three-file patch (stable patch IDc0a90acc3bcf3719fd4c3a07a3d3d1efc4142287).Production delta: +6 lines across existing lifecycle owners; tests: +152. This connects the existing classifier at the owning boundaries and removes a redundant no-result branch. No new abstraction or persisted state is introduced. No visual layout changes. Cloud startup latency and macOS display availability remain separate follow-ups, not claims of this repair.