Skip to content

fix: Companion session cards open the main session instead of the selected one - #1436

Open
yadiaprianto wants to merge 1 commit into
openclaw:mainfrom
yadiaprianto:fix/companion-chat-deeplink-chat-route
Open

yadiaprianto wants to merge 1 commit into
openclaw:mainfrom
yadiaprianto:fix/companion-chat-deeplink-chat-route

Conversation

@yadiaprianto

Copy link
Copy Markdown

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 /chat keeps 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 /chat because it needs no session-key parsing. Happy to switch the Companion to the canonical path form if you would rather emit it.

Evidence

  • Reproduced headlessly against a running Gateway with a fresh browser profile: /?token=...&session=<old key> renders the main session; /chat?token=...&session=<old key> renders the requested session.
  • The same URL change is covered by unit tests on GatewayChatUrlBuilder.
Focused test output
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~GatewayChatHelperTests"
Passed: 14, Failed: 0, Total: 14

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required 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 at Windows 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, two TokenSanitizerTests cases, 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 (expected Last 10:30:45, host culture formats the time differently) and two NativeSpeechStackRuntimeTests cases (this host has no Visual Studio VC++ Redistributable at or above 14.38.33130, so the app-local msvcp140.dll is 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

  • Environment tested: Windows 11 (10.0.26200, x64), .NET SDK 10.0.401, Node v24.19.0.
  • PR head or commit tested: ed96f0c
  • Exact steps or command run: dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~GatewayChatHelperTests"
  • Evidence after fix: the builder now produces http://127.0.0.1:18789/chat?token=<token>&session=<encoded key>; the three added tests assert the /chat path, the exact deep link for a session key, and that no root-path deep link is emitted.
  • Observed result: all 14 builder tests pass.
  • Screenshot or artifact links verified? N/A
  • Not verified or blocked: the in-app click-through (Companion Sessions page to WebView chat) could not be run on this host because the WinUI app cannot be built without the Windows 10 SDK. Also could not reach the live /?token=... vs /chat?token=... render comparison on this host; that comparison was observed earlier on a machine with a running Gateway. windows-winui-interactive proof is declared and not yet run.

Security Impact

  • New permissions or capabilities? No
  • Secrets or tokens handling changed? No
  • New or changed network calls? No
  • Command or tool execution surface changed? No
  • Data access scope changed? No
  • If any answer is Yes, explain the risk and mitigation: The token is still passed as a query parameter exactly as before; only the path segment changes.

Compatibility and Migration

  • Backward compatible? Yes
  • Config or environment changes? No
  • Migration needed? No
  • If yes, list the exact upgrade steps: None. Older Gateways that serve the Control UI at /chat behave the same; the route has been stable.

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

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.
@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 17, 2026
@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 17, 2026, 9:34 AM ET / 13:34 UTC.

ClawSweeper review

What this changes

The PR directs Companion’s legacy WebView chat links to /chat and adds tests covering session links, encoding, and links without a selected session.

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
Reviewed head: ed96f0cef6a848dde75e081ed410b33f9bb5d67b

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and source-supported, but its current-head runtime evidence is too thin for merge.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The changed builder feeds ChatPage’s actual WebView navigation, but supplied current-head evidence exercises only URL unit tests. The earlier Gateway comparison is useful context; the body explicitly leaves current-head Companion click-through and windows-winui-interactive proof unverified. 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.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The changed builder feeds ChatPage’s actual WebView navigation, but supplied current-head evidence exercises only URL unit tests. The earlier Gateway comparison is useful context; the body explicitly leaves current-head Companion click-through and windows-winui-interactive proof unverified. 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 8 items Pinned change and current release: The introduced delta contains only the URL builder and its tests. Current main still builds a root-path URL; the local v2026.9.4 tag points to that main revision. The requested route change is not already present there.
Production session handoff: SessionsPage stores the selected key before navigating to chat. ChatPage consumes it in legacy mode and appends it immediately before WebView navigation; both its initial URL and subsequent resolved URLs use the changed builder.
Dependency contract signal: The changed URL is navigated by WebView into the Gateway Control UI, and the PR explicitly relies on that UI accepting the session query at /chat. This establishes an actual dependency on OpenClaw’s Control UI routing contract.
Findings None None.
Security None None.

How this fits together

Companion 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]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The changed builder feeds ChatPage’s actual WebView navigation, but supplied current-head evidence exercises only URL unit tests. The earlier Gateway comparison is useful context; the body explicitly leaves current-head Companion click-through and windows-winui-interactive proof unverified. 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.
  • Resolve merge risk (P1) - The supplied evidence does not yet establish that the current-head Companion build opens the selected conversation through the actual WebView and Gateway.
  • Complete next step (P2) - Complete required Windows validation and add current-head isolated Companion proof that a non-main session card opens its matching conversation. Prefer a screenshot or recording with session diagnostics; redacted logs or copied live output also count. Remove private addresses, tokens, and conversation content. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +8/-2 lines; tests +53/-1 lines The production change remains focused on the route correction, with most added lines providing regression assertions.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep URL construction in the existing shared helper and use the accepted chat-route query form while preserving session handoff, escaping, and transport restrictions.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This repairs selected-session navigation in the optional legacy WebView surface with a limited blast radius.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The changed builder feeds ChatPage’s actual WebView navigation, but supplied current-head evidence exercises only URL unit tests. The earlier Gateway comparison is useful context; the body explicitly leaves current-head Companion click-through and windows-winui-interactive proof unverified. 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.

Label justifications:

  • P2: This repairs selected-session navigation in the optional legacy WebView surface with a limited blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The changed builder feeds ChatPage’s actual WebView navigation, but supplied current-head evidence exercises only URL unit tests. The earlier Gateway comparison is useful context; the body explicitly leaves current-head Companion click-through and windows-winui-interactive proof unverified. 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

What I checked:

  • Pinned change and current release: The introduced delta contains only the URL builder and its tests. Current main still builds a root-path URL; the local v2026.9.4 tag points to that main revision. The requested route change is not already present there. (src/OpenClaw.Tray.WinUI/Helpers/GatewayChatUrlBuilder.cs:49, 3c43751b2bac)
  • Production session handoff: SessionsPage stores the selected key before navigating to chat. ChatPage consumes it in legacy mode and appends it immediately before WebView navigation; both its initial URL and subsequent resolved URLs use the changed builder. (src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs:620, ed96f0cef6a8)
  • Dependency contract signal: The changed URL is navigated by WebView into the Gateway Control UI, and the PR explicitly relies on that UI accepting the session query at /chat. This establishes an actual dependency on OpenClaw’s Control UI routing contract. (src/OpenClaw.Tray.WinUI/Helpers/GatewayChatUrlBuilder.cs:55, ed96f0cef6a8)
  • Control UI accepts the proposed link form: The pinned upstream releasedSessionQuery implementation accepts session identity at the chat or dashboard route roots and rejects other paths. This supports /chat?session= without requiring Companion to parse session keys into canonical path segments. (ui/src/app/released-session-query.ts:26, aac3b2773c7f)
  • Captured contributor proof and validation: The complete supplied body at sourceRevision a190e58fb114b92be645ccbfb735d5bbee335d37d3d172ee716534506bb80ada reports 14 passing focused tests and an earlier headless Gateway observation. It explicitly states that current-head in-app click-through and the live URL comparison were not run. The build was blocked by the Windows SDK, while Shared and Tray suites reported four and three failures respectively. These are contributor-reported results; this read-only review ran no builds or tests. (ed96f0cef6a8)
  • Applicable proof policy: The full root AGENTS.md and required proof-validation skill were read. No additional ancestor AGENTS.md files or maintainer-notes directory were found for the changed paths. The declared windows-winui-interactive pool requires current-head evidence from the isolated WinUI app. (docs/PROOF_POOLS.md:21, ed96f0cef6a8)

Likely related people:

  • indierawk2k2: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • karkarl: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add current-head isolated WinUI evidence showing a non-main session card opens the matching conversation through the real Gateway.
  • Complete the required build and Shared/Tray validation on a suitably provisioned Windows host and report the results.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant