Skip to content

Simplify session status presentation#1057

Draft
RomneyDa wants to merge 1 commit into
mainfrom
session-status-presentation
Draft

Simplify session status presentation#1057
RomneyDa wants to merge 1 commit into
mainfrom
session-status-presentation

Conversation

@RomneyDa

Copy link
Copy Markdown
Member

Summary

  • Add a shared SessionRunState adapter that combines Gateway run outcome with hasActiveRun, with terminal outcomes taking precedence over stale liveness.
  • Present only three primary statuses: Working, Ready, and Needs attention. A stopped run is secondary context, not a fourth badge.
  • Hide only successful completed sessions by default; keep failed, timed-out, stopped, and reusable sessions visible and available in the chat picker.
  • Use the same rule in Sessions, Connection, tray dashboard, tray menu, and session tests.

Validation

  • Passed: git diff --check, git show --check, and XML/resource-key parity across 5 locales.
  • Blocked: ./build.ps1 cannot run on this macOS host (no PowerShell); both required dotnet test commands are blocked because dotnet is not installed.
  • Blocked: remote Windows validation could not start because Crabbox has no configured coordinator/lease.
  • Review helper was invoked in local and branch modes, but this controller returned no report or artifact; no automated-review pass is claimed.

Real behavior proof

Not verified / blocked: no native Windows runtime, .NET SDK, PowerShell, or authenticated remote Windows lease is available in this controller. The PR includes focused shared-state, parser, visibility, dashboard, and WinUI source-contract tests for execution in CI or a Windows environment.

User-facing model

Display state Meaning
Working A run is currently active
Ready The session can receive the next turn
Needs attention The last run failed or timed out

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 26, 2026
@clawsweeper

clawsweeper Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 26, 2026, 12:42 AM ET / 04:42 UTC.

ClawSweeper review

What this changes

The PR adds shared interpretation of Gateway session outcome and liveness data, changes tray/session surfaces to Working, Ready, or Needs attention, and hides only successful completed sessions by default.

Merge readiness

⚠️ Ready for maintainer review - 6 items remain

Keep this draft PR open for maintainer review. Its three-state presentation is coherent, but the new adapter treats a documented live-run signal as stale whenever an older terminal status remains, which can present an active session as Ready or Needs attention.

Priority: P2
Reviewed head: 6db917b52fbfc5aaec41e1262391d02e47991bee
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The PR has focused coverage and a coherent presentation goal, but an adapter-level contract conflict and missing required Windows validation prevent a higher rating.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored draft is not subject to the external-contributor proof gate, but repository policy still requires current-head Windows build, test, and visible tray validation before merge.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored draft is not subject to the external-contributor proof gate, but repository policy still requires current-head Windows build, test, and visible tray validation before merge.
Evidence reviewed 4 items Gateway liveness contract: The public Gateway client guidance tells clients to read sessionInfo.hasActiveRun for current runtime activity and explains that a true value can represent an active runtime projection. That conflicts with the PR's unconditional terminal-status precedence.
Conflicting precedence in proposed adapter: The proposed IsWorking method returns false for completed, failed, stopped, and timed-out status before evaluating HasActiveRun, and the new test explicitly locks in failed plus true as not working.
Affected product surfaces: The adapter is used by session visibility, chat thread state, the Connection page, tray dashboard, and tray menu, so an incorrect active-run result propagates beyond the Sessions page.
Findings 2 actionable findings [P2] Honor a true live-run signal before terminal status
[P2] Run the required Windows validation before merge
Security None None.

How this fits together

Gateway session snapshots feed the Windows tray, session picker, connection card, and dashboard. The shared session-state adapter converts Gateway outcome and liveness fields into display state, visibility, ordering, and chat-thread status.

flowchart LR
  Gateway[Gateway session snapshot] --> Parser[Windows Gateway parser]
  Parser --> State[Shared session-state adapter]
  State --> Sessions[Sessions page and chat picker]
  State --> Connection[Connection card]
  State --> Tray[Tray dashboard and menu]
  Sessions --> User[Operator session actions]
  Connection --> User
  Tray --> User
Loading

Decision needed

Question Recommendation
Should the Windows client follow the Gateway contract by treating hasActiveRun: true as authoritative for current activity, with terminal status used only when liveness is absent? Use live-run authority: Treat hasActiveRun: true as Working and use terminal outcome only as fallback or secondary context when the liveness field is missing.

Why: The proposed terminal-first policy intentionally changes the Gateway signal precedence and affects multiple user-facing session surfaces; the existing public Gateway contract favors the opposite interpretation.

Before merge

  • Honor a true live-run signal before terminal status (P2) - hasActiveRun is the Gateway's explicit current-runtime signal. With status: "failed" from a prior run and hasActiveRun: true for a new run, this branch returns false before it reads liveness, so every consumer renders the session as non-working. Make a true liveness value authoritative, then use terminal status only when liveness is unavailable or as secondary context.
  • Run the required Windows validation before merge (P2) - This shared behavior changes Gateway parsing and several WinUI/tray surfaces. Repository policy requires ./build.ps1 plus the shared and tray test projects after each code change; the PR body reports those checks were blocked. Please provide the required run from a Windows-capable environment after resolving the current merge conflict.
  • Resolve merge risk (P1) - A Gateway session with an active current run but a retained terminal outcome can be displayed as Ready or Needs attention, which can hide in-flight work across the session picker, connection card, dashboard, and tray menu.
  • Resolve merge risk (P1) - The PR currently has merge conflicts and lacks the repository-required Windows build and test validation, so its behavior needs rechecking after rebase.
  • Complete next step (P2) - Resolve the Gateway liveness contract and current merge conflict before the required Windows validation can establish a safe merge verdict.

Findings

  • [P2] Honor a true live-run signal before terminal status — src/OpenClaw.Shared/Sessions/SessionRunState.cs:47-51
  • [P2] Run the required Windows validation before merge — src/OpenClaw.Shared/Sessions/SessionRunState.cs:1
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 22 files affected, 386 additions, 113 deletions A shared state rule is propagated through Gateway parsing, session views, tray summaries, localization, documentation, and tests.
Localized presentation 5 locale resource files changed The new status labels are user-facing across every supported locale.

Merge-risk options

Maintainer options:

  1. Correct live-run precedence before merge (recommended)
    Change the shared adapter so a true Gateway liveness field remains Working, add conflict-state regression coverage, then rebase and run the required Windows validations.
  2. Accept terminal-first semantics explicitly
    Keep the current policy only if maintainers confirm the Gateway can emit stale true liveness and the PR documents and proves that compatibility contract.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Treat `hasActiveRun: true` as Working; preserve terminal status as fallback only when liveness is unavailable, add regression coverage for conflicting fields, then rebase and run the required build and test subset.

Technical review

Best possible solution:

Make a true hasActiveRun authoritative for active-run presentation, use terminal status as the fallback when liveness is absent, update the conflicting tests, then rebase and validate the complete Windows tray flow.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: construct a session with a terminal status such as failed and HasActiveRun = true. The proposed adapter returns not working even though the Gateway client contract describes a true liveness flag as an active runtime projection.

Is this the best way to solve the issue?

No. The terminal-first rule is not the safest default because it overrides the Gateway's explicit live-run field; liveness should be authoritative when supplied, with status used as fallback or secondary outcome context.

Full review comments:

  • [P2] Honor a true live-run signal before terminal status — src/OpenClaw.Shared/Sessions/SessionRunState.cs:47-51
    hasActiveRun is the Gateway's explicit current-runtime signal. With status: "failed" from a prior run and hasActiveRun: true for a new run, this branch returns false before it reads liveness, so every consumer renders the session as non-working. Make a true liveness value authoritative, then use terminal status only when liveness is unavailable or as secondary context.
    Confidence: 0.88
  • [P2] Run the required Windows validation before merge — src/OpenClaw.Shared/Sessions/SessionRunState.cs:1
    This shared behavior changes Gateway parsing and several WinUI/tray surfaces. Repository policy requires ./build.ps1 plus the shared and tray test projects after each code change; the PR body reports those checks were blocked. Please provide the required run from a Windows-capable environment after resolving the current merge conflict.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f50af187865c.

Labels

Label changes:

  • add P2: The patch can misrepresent active session state across common Windows operator surfaces, but the evidence does not show an emergency runtime outage.
  • add merge-risk: 🚨 compatibility: The PR changes established session visibility and status interpretation for existing Gateway data, including the precedence of persisted outcome versus live-run state.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored draft is not subject to the external-contributor proof gate, but repository policy still requires current-head Windows build, test, and visible tray validation before merge.

Label justifications:

  • P2: The patch can misrepresent active session state across common Windows operator surfaces, but the evidence does not show an emergency runtime outage.
  • merge-risk: 🚨 compatibility: The PR changes established session visibility and status interpretation for existing Gateway data, including the precedence of persisted outcome versus live-run state.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored draft is not subject to the external-contributor proof gate, but repository policy still requires current-head Windows build, test, and visible tray validation before merge.

Evidence

What I checked:

  • Gateway liveness contract: The public Gateway client guidance tells clients to read sessionInfo.hasActiveRun for current runtime activity and explains that a true value can represent an active runtime projection. That conflicts with the PR's unconditional terminal-status precedence.
  • Conflicting precedence in proposed adapter: The proposed IsWorking method returns false for completed, failed, stopped, and timed-out status before evaluating HasActiveRun, and the new test explicitly locks in failed plus true as not working. (src/OpenClaw.Shared/Sessions/SessionRunState.cs:47, 6db917b52fbf)
  • Affected product surfaces: The adapter is used by session visibility, chat thread state, the Connection page, tray dashboard, and tray menu, so an incorrect active-run result propagates beyond the Sessions page. (src/OpenClaw.Tray.WinUI/Services/SessionVisibilityFilter.cs:14, 6db917b52fbf)
  • Repository validation policy: The repository policy requires the full build plus shared and tray tests after every code change. The PR body reports these were blocked, so a Windows validation run is still needed before merge. (AGENTS.md:1, f50af187865c)

Likely related people:

  • RomneyDa: The provided PR context identifies this repository member as the author of the focused session-state and UI change; the current-main ownership trail could not be independently completed in this read-only review environment. (role: recent session-presentation contributor; confidence: low; commits: 6db917b52fbf; files: src/OpenClaw.Shared/Sessions/SessionRunState.cs, src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml.cs)

Rank-up moves

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

  • Make a true hasActiveRun authoritative and update the conflicting regression tests.
  • Resolve the dirty merge state, then run ./build.ps1 and both required test projects on Windows.
  • Capture current-head WinUI/tray evidence for Working, Ready, Needs attention, and default visibility behavior, with any private endpoints or tokens redacted.

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

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant