Repository navigation
fix(android): retire resolved approval cards with their result - #140111
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: blocked before merge. Reviewed September 6, 2026, 8:59 AM ET / 12:59 UTC. ClawSweeper reviewWhat this changesAndroid approval results, pending cards, and counts now share one observable snapshot so resolved requests disappear when their outcome appears. Merge readiness⛔ Blocked before merge - 3 items remain This remains a useful, focused fix: current main still permits inconsistent approval presentation, and no introduced correctness defect was found. The linked merged Android changes address different problems. Priority: P2 Review scores
Verification
How this fits togetherAndroid receives pending command approvals and terminal results from the Gateway. Its runtime supplies the approval screen, Home counts, and Wear summary while the existing decision path submits authorized responses. flowchart TD
A[Gateway approval events and responses] --> B[Lifecycle and refresh checks]
B --> C[Android approval inbox snapshot]
C --> D[Pending cards and result banner]
C --> E[Home and Wear counts]
D --> F[User approval decision]
F --> G[Existing Gateway decision checks]
Before merge
Agent review detailsSecurityNone. PR surfaceTests +296, Other +12. Total +308 across 8 files. View PR surface stats
Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep approval rows and terminal outcomes under one runtime-owned snapshot while preserving the existing lifecycle, recovery, and decision checks. Do we have a high-confidence way to reproduce the issue? Yes: current main exposes the separate notice/row publication sequence, and the supplied controlled Compose reproduction targets that exact boundary. This review did not execute it. Is this the best way to solve the issue? Yes: consolidating publication at the runtime owner removes the inconsistency across consumers without adding UI timing workarounds or changing approval policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 4418cdcc5875. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
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.
Closes #140109. Related: #134939 and #139789.
What Problem This Solves
Fixes an issue where users reviewing Android command approvals could see “already denied” while the same request still showed enabled Allow/Deny controls and counted as pending. A controlled real-Compose/loopback reproduction captures this contradictory frame on unchanged production.
Why This Change Was Made
NodeRuntime previously published rows, refresh/error state, and terminal notices through four independent StateFlows. MainViewModel and Compose collected them separately, so a writer lock could not provide an atomic UI snapshot.
One internal immutable approval inbox now owns those facts. All six terminal-producing paths retire the matching card and publish its result together. Settings, Home, and Wear read that coherent state; the old parallel flows are removed. Conditional dismissal preserves replacement notices, and existing refresh generations, Gateway lifecycle/method checks, ambiguous-write recovery, expiry, and decision authorization remain intact.
User Impact
A resolved request disappears from the pending list when its outcome appears. Other pending requests remain actionable and the result stays visible until dismissed. No configuration, dependency, protocol, storage/schema, or permission change is introduced. This repairs the existing documented approval behavior rather than adding a new approval policy.
Evidence
efdd37fed1a9dcca63b377ac54d456641b55ee0c. Its rebase onto0a753eb92f6f687476654f68c12143b68ea804fapreserves all eight reviewed/tested afterimages byte-for-byte; current-main Android drift is four unrelated generated protocol method names.Before and after
These are synthetic Compose/Robolectric captures under controlled scheduling, inspected in full by the maintainer agent. They are not installed-device screenshots. Refreshing legitimately remains visible while the publisher is paused.
Validation and limits
Exact-head hosted CI 34034559425 passed, including Play/third-party/Wear tests, Play/Wear builds, ktlint, and the required CI gate. The installed API 36 Play/debug flow also passed: fresh install and manual TLS onboarding, Pending 1 with enabled controls, remote denial followed by canonical
approval.get, Pending 0 with the matching notice and no approval card, Dismiss, and ordinary Disconnect. Installed APK bytes match reviewedefdd37fed1a9dcca63b377ac54d456641b55ee0c(SHA-2568ce416b74867fd592efb874aaf01a16f7ab19e51a7f5fbdc3c6871f3a5a3a16a). No approval resolution/request or chat-send RPC occurred. Display and locale remained unchanged. Complete evidence was downloaded; native processes/ports closed and the Testbox was stopped. The regression measures controlled rendered inconsistency, not natural recurrence frequency, duplicate execution, or an authorization bypass. The first candidate witness stopped on Refresh readiness; that failed attempt is retained, and the final old/new controls use the same explicit rendered-ready condition without increasing its timeout.Installed-app sequence
These unmodified full-frame captures are from the installed app using an isolated synthetic Gateway fixture, personally inspected by the maintainer agent. They show the ordinary fixed-app flow, not a controlled scheduling reproduction or a production Gateway authorization test.
Review disposition
The latest ClawSweeper review reported no source/security findings and requested completion of the declared installed flow. That flow is now complete; its image-fetch limitation is addressed by direct full-frame inspection of both the existing controlled Compose captures and the installed sequence above. This is the maintainer agent’s evidence review, not a claim of separate human review. The reviewed owner-boundary design is unchanged. Two earlier large-input sync attempts failed before native execution; both leases were stopped. The successful run used ordinary remote builds and local-on-Testbox archive assembly without changing providers, timeout guards, or transport behavior.
Post-merge integration audit
The landed tree preserves the reviewed runtime changes and the unrelated provider-refresh work on main. The audit also caught two test-only references which moving main had added to the removed getters; #140247 repairs that clean-merge composition. Its compiler reproduction and 30-test validation are recorded there.