fix: Companion session cards open the main session instead of the selected one - #1436
yadiaprianto wants to merge 1 commit into
Conversation
The legacy WebView chat surface built its URL against the Gateway root (`/?token=...&session=...`). The Control UI honours the released `?session=` identity only at the chat route root, so at `/` it drops the parameter and restores the browser's last selected session. Every Sessions card "Open chat" click therefore opened the main session instead of the requested one. Build the deep link against the `/chat` route instead, keeping the token and the optional `session` parameter unchanged. Verified against a running Gateway: `/?token=...&session=<key>` renders the main session, while `/chat?token=...&session=<key>` renders the requested session. Tests: GatewayChatHelperTests now assert the `/chat` path, the exact deep link produced for a session key, and that the old root-path form is not emitted.
|
🦞👀 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. |
|
Codex review: needs real behavior proof before merge. Reviewed September 17, 2026, 9:34 AM ET / 13:34 UTC. ClawSweeper reviewWhat this changesThe PR directs Companion’s legacy WebView chat links to Merge readiness⛔ Blocked before merge - 3 items remain This is a focused, still-useful fix for legacy WebView session links. Source inspection supports the approach and found no introduced correctness defect, but current-head runtime proof remains incomplete. Priority: P2 Review scores
Verification
How this fits togetherCompanion session cards hand a selected session key to the chat page. In legacy WebView mode, the page combines that key with a Gateway URL and token, then opens the Gateway’s Control UI. flowchart TD
A[Session card] --> B[Pending session key]
C[Gateway address and token] --> D[Chat URL builder]
D --> E[Chat route URL]
B --> F[WebView navigation]
E --> F
F --> G[Control UI selected conversation]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep URL construction in the existing shared helper and use the accepted chat-route query form while preserving session handoff, escaping, and transport restrictions. Do we have a high-confidence way to reproduce the issue? Yes, source establishes a concrete path: select a session card with legacy WebView enabled, then observe the root-path session link emitted by main. The inspected Control UI parser accepts that identity at /chat; this review did not execute the app. Is this the best way to solve the issue? Yes, changing the shared URL builder is a narrow repair, and the inspected Control UI contract accepts the proposed query form. Actual Companion click-through still needs current-head confirmation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 3c43751b2bac. 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
|
What Problem This Solves
Fixes: opening a chat from the Companion Sessions page shows the main session instead of the session on the card, for anyone using the legacy WebView chat surface.
User Impact
User impact: clicking "Open chat" on a session card now opens that session. The legacy WebView chat surface deep-links to the Control UI chat route, so the card's session key is honored instead of being dropped.
Why This Change Was Made
The WebView chat URL was built against the Gateway root:
/?token=...&session=.... The Control UI accepts the released?session=identity only at the chat route root, so at/it drops the parameter and restores the browser's last selected session, which is normally the main session. Pointing the URL at/chatkeeps the token and session parameters unchanged and lets the Control UI apply the requested session.Note on link form: the Control UI URL contract prefers canonical
/chat/<agentId>/<rest>path links for new links, and documents?session=as a migration aid accepted at the application boundary. I could not find a maintainer preference recorded in this repo, so this change uses the accepted?session=form at/chatbecause it needs no session-key parsing. Happy to switch the Companion to the canonical path form if you would rather emit it.Evidence
/?token=...&session=<old key>renders the main session;/chat?token=...&session=<old key>renders the requested session.GatewayChatUrlBuilder.Focused test output
Change Type
Scope
winnodeRequired proof pools
windows-winui-interactive: the legacy WebView chat deep link is tray UX and needs a current-head visual check in the isolated app.Validation
Baseline commands:
./build.ps1: blocked on this host. The prerequisite check stops atWindows 10 SDK not found (needed for WinUI build), so the WinUI app cannot be built here. Documentation validation runs first and passes (48 Markdown files checked, proof-pool validation passed).dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj: Failed: 4, Passed: 3977, Skipped: 35, Total: 4016. The 4 failures are in untouched areas (MarkdownParserFuzzTests.Build_ManyListItems_ScalingCurve, twoTokenSanitizerTestscases,McpHttpServerTests.Dispose_DuringInFlightHandler_DoesNotSurfaceObjectDisposedException). This change does not compile into that project.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj: Failed: 3, Passed: 2978, Total: 2981. The 3 failures are unrelated to this change:TrayTooltipBuilderTests.Build_ConnectedWithChannelsAndNodes_ContainsExpectedSegments(expectedLast 10:30:45, host culture formats the time differently) and twoNativeSpeechStackRuntimeTestscases (this host has no Visual Studio VC++ Redistributable at or above 14.38.33130, so the app-localmsvcp140.dllis the 14.29 fallback and onnxruntime cannot load).Focused tests for this change:
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~GatewayChatHelperTests": Passed: 14, Failed: 0, Total: 14.Real Behavior Proof
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~GatewayChatHelperTests"http://127.0.0.1:18789/chat?token=<token>&session=<encoded key>; the three added tests assert the/chatpath, the exact deep link for a session key, and that no root-path deep link is emitted./?token=...vs/chat?token=...render comparison on this host; that comparison was observed earlier on a machine with a running Gateway.windows-winui-interactiveproof is declared and not yet run.Security Impact
Compatibility and Migration
/chatbehave the same; the route has been stable.Review Conversations