Add Default reasoning reset to chat - #1077
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 5, 2026, 8:44 PM ET / August 6, 2026, 00:44 UTC. ClawSweeper reviewWhat this changesThis draft adds a Default reasoning menu action that clears a chat session’s explicit reasoning override and reconciles Gateway-confirmed session state. Merge readinessKeep open. Current main still presents an unset reasoning level as medium and has no user action to clear it; this collaborator-authored draft is an active three-layer stack whose dependencies and current-head proof remain unresolved. Priority: P2 Review scores
Verification
How this fits togetherThe tray chat composer sends reasoning choices through the chat provider and bridge to a Gateway session patch. Gateway responses and session snapshots then determine the reasoning state shown back in chat. flowchart LR
A[Chat reasoning menu] --> B[Chat provider]
B --> C[Gateway bridge]
C --> D[Gateway session patch]
D --> E[Gateway response]
E --> F[Session snapshot]
F --> G[Displayed reasoning state]
Decision needed
Why: The change adds a user-visible reasoning mode and response-aware session API path across a three-PR stack; acceptance requires maintainer judgment beyond mechanical review. Before merge
Agent review detailsSecurityNone. 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: Approve or defer the Default semantics, then land the dependency layers in order; if approved, restack this draft onto main and prove an explicit off-to-Default restore through an isolated real Gateway plus a current-head WinUI capture. Do we have a high-confidence way to reproduce the issue? No for the full Gateway behavior in this review. Current main source clearly lacks the Default action, but an exact model and real-Gateway run was unavailable and the PR’s own current-head proof is blocked. Is this the best way to solve the issue? Unclear. Using the existing null SessionPatch representation is a narrow fit, but the response-aware reconciliation stack and cross-Gateway behavior need maintainer sponsorship and current-head proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 90df4a4af995. LabelsLabel 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
HistoryReview history (24 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
c4f48b6 to
83386c8
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@clawsweeper re-review |
Related to #980
Stack
This is Layer 3 of a three-PR stack:
79b289e0d7d91e05a6f7da507d3cf25ec9927dfd:PendingRequestRegistryowns request correlation and response classification.6480e8702f9bebe94cffbc2423bc3e60a3092b55:ThinkingLevelClearReconcilerowns reasoning-clear operation, generation, ACK, retry, supersession, and disposal state.96549f9766e5e30812482251de75d76d628ea23a: adds the focused Windows Default/reset behavior, provider delegation and cache convergence, architecture closure guards, typed protocol, proof, and tests.This PR remains draft and must not merge before its dependencies.
Root cause
Choosing any concrete reasoning level, including
off, persisted a sessionThinkingLeveloverride. The tray exposed only concrete levels and labeledmediumas the default, even though the Gateway protocol supports clearing a session field throughSessionPatch.Clear.After a clear was confirmed, the bridge cache could still replay a stale concrete value during same-provider reload/remount. That stale cache row could overwrite the provider's confirmed canonical null even though the Gateway remained on Default. Separately, a rejected or interrupted clear could leave the reconciler confirmation task faulted after the caller stopped awaiting it.
Behavior
offand every concrete level.mediumis no longer labeled as the default.thinkingLevel: null. It never sends"default"or substitutes"medium".sessions.patchACK and a current-generation, response-correlatedsessions.listsnapshot confirm null.Final Layer 3 corrections
OpenClawChatDataProviderkeeps a provider-instance, per-thread authority token after an accepted correlated null confirmation. Ordinary bridge/cache rows and non-target correlated rows clone and mask only stale non-nullThinkingLevel; they never mutate bridge-owned objects. Tokens retire only on ordered evidence: ordinary/cache null, row disappearance, connected-authority transition, provider disposal/recreation, the thread's correlated confirmed non-null, or a successful later concrete selection that still owns the exact captured token. A newer clear cannot be retired by an older concrete operation.ClearOperation.Confirmation. Await behavior, surfaced errors, timeout behavior, and cancellation semantics remain unchanged, while direct rejection and post-ACK disconnect faults cannot escape as matchingUnobservedTaskException.Ownership and compatibility
OpenClawComposerandOpenClawReactorChatRootown the Default menu state and accessible checked semantics.OpenClawChatRootandOpenClawChatDataProvideradapt typed user intent and canonical session updates.ThinkingLevelClearReconcilerremains the sole owner of reasoning-clear lifecycle state. The provider does not duplicate operation maps, retry timers, generation state, or cancellation ownership.PendingRequestRegistryremains the sole owner of response correlation. The Gateway client does not add feature-specific request maps or completed-ID stores.IChatGatewayBridgeandOpenClawGatewayClientown the typed protocol boundary and exact wire payload.sessions.patchclear contract. Unknown or unsupported methods surface a truthful error and retain the prior selection.OpenClawChatDataProvider; Shared, Gateway, reconciler, root, coordinator, and public API surfaces are unchanged.Upstream core capability and model-validation work remains separate and out of scope: openclaw/openclaw#99240, openclaw/openclaw#99798, and openclaw/openclaw#109623.
Validation
Validated on exact Layer 3 head
96549f9766e5e30812482251de75d76d628ea23awithOPENCLAW_REPO_ROOTset to the isolated worktree:git diff --check.\build.ps1dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.WinNode.Cli.Tests\OpenClaw.WinNode.Cli.Tests.csproj --no-restoreOPENCLAW_RUN_E2EunsetFocused and stress validation
The final readiness audit reapproved this exact two-file diff with high confidence and zero findings after the one accepted independent-review issue was fixed and all required validation was rerun.
Real behavior proof
Exact-head automated evidence
Exact-head deterministic tests prove:
LoadAsync/remount reads remain null even while the bridge cache is stale.offis masked; ordinary null releases authority; later concrete canonical evidence is accepted.SessionInfoobjects are not mutated, while unrelated fresh fields remain authoritative.The loopback protocol proof retains the typed clear request shape:
{"type":"req","id":"<request-id>","method":"sessions.patch","params":{"key":"agent:main:main","thinkingLevel":null}}It also asserts the frame does not contain
"thinkingLevel":"default"or"thinkingLevel":"medium".Hosted exact-head proof
workflow_dispatchentry and pull-request CI selects only PRs targetingmainormaster. This stacked draft targets Layer 2, so hosted Build/Test and the real reasoning Gateway E2E cannot be dispatched for this exact stack head without changing workflows.Exact-head media
Not verified / blocked. The current controller does not expose an active interactive screen-capture handle shared with the app. The prior normal-profile attempt on historical head
83386c86ec15546a17aeb0d293bc4e23034c1b3drestored Default butGraphics.CopyFromScreenfailed withThe handle is invalid; no blank or unsafe frame was uploaded. Exact-head capture was not attempted without a privacy-safe interactive desktop.The images below are historical UI references only, captured from prior Layer 3 head
c4f48b62c7f3729ade8293c77f9a50f3911ecaddwith an isolated temporary WinUI test host and synthetic props. They are not current-head proof.Historical canonical null, with Default checked:
Historical explicit
off, with off checked and Default independently available:73485452c52ea9d67d64f43ed135ea159cec3617d357270be34673c9f132ee40ce79bdf3327a92cb1fc80a08ff63834d83280f2f7eed7d3695f4c62460c556d8Real Gateway
Not verified / blocked. Local WSL/setup/real-Gateway proof was intentionally not run because this host has a recorded isolation-cleanup incident. No credentials, providers/models, real chats, or user sessions were used.
SessionsPatchThinkingLevelE2ETestsremains the credential-free isolated proof for explicitoff-> canonical null -> Tray reconnect convergence with state restoration infinally. The workflow selects it only when the finalized stack targetsmain, so exact-head execution remains blocked until dependencies land or a verified disposable remote is available.Residual proof gap: behavior against an older or lossy real Gateway under real network timing remains unverified. Unsupported-method, timeout, cancellation, delayed-response, remount, and race paths are covered deterministically.
Review
96549f9766e5e30812482251de75d76d628ea23a.PatchSessionThinkingLevelAsynccompletes after outboundSendRawAsync, not after the Gateway response. Gemini re-reviewed that evidence and closed clean at 99%.ClawSweeper must re-review this exact head after the body and hosted proof links are final. Target gate: zero code/security findings and sufficient proof.
Maintainer-only cap: this draft cannot merge until dependent draft PRs #1090 and #1094 land and the stack is finalized. This PR will remain open and draft.