Skip to content

Fix WebChat dispatch failure session status#84352

Merged
jesse-merhi merged 2 commits into
mainfrom
jesse/fix-webchat-dispatch-session-status
Jun 23, 2026
Merged

Fix WebChat dispatch failure session status#84352
jesse-merhi merged 2 commits into
mainfrom
jesse/fix-webchat-dispatch-session-status

Conversation

@jesse-merhi

@jesse-merhi jesse-merhi commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

WebChat dispatch failures before an agent run starts now close the persisted session lifecycle instead of leaving the session stuck as running. The gateway already broadcast a chat error and removed active-run tracking; this PR adds the missing terminal session lifecycle write and sessions.changed notification so sessions.list and Control UI consumers see status: "failed" with hasActiveRun: false.

The current net diff is limited to src/gateway/server-methods/chat.ts and src/gateway/server.chat.gateway-server-chat.test.ts.

What Changed

  • Dispatch rejection handling: chat.send now records a pending lifecycle error only when dispatch rejects before agentRunStarted, the run was not explicitly aborted, and the run is not in chatAbortedRuns.
  • Session lifecycle persistence: after active-run cleanup, the gateway persists a terminal lifecycle event with phase: "error" when no active run remains for the session.
  • Session change notification: after successful lifecycle persistence, the gateway emits sessions.changed with reason chat.dispatch-error so session subscribers receive the failed terminal state.
  • Regression coverage: the WebChat gateway test now rejects dispatchInboundMessage, waits for the chat error and sessions.changed, and verifies both the event and sessions.list report status: "failed" and hasActiveRun: false.

Flow

flowchart TD
  A["WebChat chat.send starts"] --> B["dispatchInboundMessage rejects before agent run starts"]
  B --> C["Broadcast chat error"]
  C --> D["Clean active run tracking"]
  D --> E{"Any active run still tracked for session?"}
  E -- "yes" --> F["Leave session lifecycle unchanged"]
  E -- "no" --> G["Persist lifecycle phase: error"]
  G --> H["Emit sessions.changed with failed status"]
Loading

Proof

Behavioral proof:

  • Before this PR, the pre-agent dispatch rejection path could broadcast a chat error and remove the active run while leaving the persisted session row as running.
  • After this PR, the same path emits a session-change event like this for subscribed WebChat clients:
{
  "type": "event",
  "event": "sessions.changed",
  "payload": {
    "reason": "chat.dispatch-error",
    "sessionKey": "agent:main:main",
    "sessionId": "sess-main",
    "status": "failed",
    "hasActiveRun": false
  }
}
  • A follow-up sessions.list call returns the same terminal shape for the affected session: status: "failed", hasActiveRun: false, and populated startedAt, endedAt, and runtimeMs fields.

Screenshots

No screenshots are included. This PR changes backend gateway session lifecycle persistence and event payloads, not Control UI layout or visible components; the reviewer-visible behavior is better proven by the sessions.changed payload and sessions.list state asserted by the regression test. A screenshot would only show unchanged UI consuming the fixed gateway state.

Verification

Reviewer-checkable behavior:

  • src/gateway/server.chat.gateway-server-chat.test.ts includes marks a running webchat session failed when dispatch rejects before a reply, which simulates the dispatch rejection and asserts the event and list-state behavior above.
  • The Mermaid diagram in this PR body was validated with mmdc before posting.

Automated checks:

  • pr_net_diff.py --markdown showed only two net changed files and no branch-only churn.
  • node scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat.test.ts -t "marks a running webchat session failed when dispatch rejects before a reply" passed on head c702c8b59b7.
  • node scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat.test.ts passed on head c702c8b59b7 with 72 tests.
  • pnpm exec oxfmt --check src/gateway/server-methods/chat.ts src/gateway/server.chat.gateway-server-chat.test.ts passed.
  • node scripts/run-oxlint.mjs src/gateway/server-methods/chat.ts passed.
  • pnpm tsgo:test:src passed.
  • git diff --check passed.
  • GitHub CI run 28010707174 passed on head c702c8b59b7, including check-lint, check-test-types, and checks-node-compact-small-5.

Not run:

  • Slack/Discord channel delivery: this PR does not change channel delivery code; it changes gateway session lifecycle state and WebChat session-event behavior.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: S maintainer Maintainer-authored PR labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 22, 2026, 8:23 AM ET / 12:23 UTC.

Summary
The PR changes chat.send dispatch-error cleanup to persist a failed session lifecycle event, emit sessions.changed, and add a WebChat Gateway regression test.

PR surface: Source +65, Tests +67. Total +132 across 2 files.

Reproducibility: yes. at source level: current main's chat.send dispatch rejection path broadcasts an error and removes the active run without persisting a failed lifecycle state. I did not run the failing scenario in this read-only review, but the current source path and added regression test make the stale running plus hasActiveRun:false shape clear.

Review metrics: 1 noteworthy metric.

  • Terminal Lifecycle Persistence: 1 added path. The PR adds a new terminal session-state write after WebChat dispatch rejection, so reviewers need final-head session-state race and broadcast proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Clear or explain the current-head check-lint and compact-shard failures with logs.
  • Confirm merge sequencing with fix(gateway): preserve abort after dispatch rejection #91000 or rebase after it.
  • Refresh the PR proof/body if maintainers want evidence tied exactly to head 51b45039412af89653e2de07b87dbf22032e05de.

Risk before merge

  • [P1] The same chat.send dispatch-rejection catch path is also owned by open fix(gateway): preserve abort after dispatch rejection #91000 for abort/error ordering, so merge order must preserve both terminal abort semantics and this failed-session persistence.
  • [P1] Current head 51b45039412af89653e2de07b87dbf22032e05de had check-lint and checks-node-compact-small-5 failing when inspected, and logs were not yet available because the workflow was still in progress.
  • [P1] The supplemental image proof visible in the PR body names the older head 58a1c5d9b9f0e5770a455cd1860e8296d17b5b46, so current-head validation should come from refreshed CI or refreshed proof before merge.

Maintainer options:

  1. Clear Final-Head Gateway Validation (recommended)
    Before merge, clear or explain the current check-lint and compact-shard failures on head 51b45039412af89653e2de07b87dbf22032e05de and keep the focused Gateway/session lifecycle tests green.
  2. Sequence The Abort-Ordering Change
    Merge after, rebase over, or deliberately combine with fix(gateway): preserve abort after dispatch rejection #91000 so this catch path does not regress terminal abort/error ordering while adding failed-session persistence.
  3. Pause For Session-State Owner Review
    If maintainers are not ready to choose the combined lifecycle semantics, hold this PR for the people closest to Gateway session lifecycle and restart-recovery behavior.

Next step before merge

  • [P2] Manual review is appropriate because the PR is maintainer-labeled and the remaining blocker is session-state merge sequencing plus final-head validation, not a narrow automated code repair.

Security
Cleared: The current diff is limited to Gateway TypeScript and a regression test, with no dependency, workflow, package, secret, permission, or artifact-execution surface changes.

Review details

Best possible solution:

Land a final head that keeps this failed-session lifecycle persistence, preserves the abort/error ordering from #91000 or rebases after it, and clears focused Gateway/session lifecycle validation.

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

Yes, at source level: current main's chat.send dispatch rejection path broadcasts an error and removes the active run without persisting a failed lifecycle state. I did not run the failing scenario in this read-only review, but the current source path and added regression test make the stale running plus hasActiveRun:false shape clear.

Is this the best way to solve the issue?

Mostly yes for the narrow WebChat pre-agent dispatch rejection case: the owner path is the chat.send catch/finally cleanup and the shared lifecycle helper already owns persisted session status. The best mergeable form still needs final-head validation and coordination with #91000 because both change the same terminal catch path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a bounded Gateway/WebChat session-state bug fix with visible stuck-running impact, but not an emergency core-runtime outage.
  • merge-risk: 🚨 session-state: The diff changes when failed WebChat dispatches write terminal session lifecycle state and broadcast active-run status.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this member-authored, maintainer-labeled PR; supplemental local logs and media are useful but the visible screenshot references a previous head, so current-head CI/proof should still be checked before merge.
Evidence reviewed

PR surface:

Source +65, Tests +67. Total +132 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 69 4 +65
Tests 1 67 0 +67
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 136 4 +132

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/gateway/server-chat.agent-events.test.ts src/gateway/server.chat.gateway-server-chat.test.ts src/gateway/session-lifecycle-state.test.ts src/gateway/server.sessions.list-changed.test.ts.
  • [P1] pnpm tsgo:test:src.
  • [P1] pnpm exec oxfmt --check src/gateway/server-methods/chat.ts src/gateway/server.chat.gateway-server-chat.test.ts.

What I checked:

  • Repository policy read: Root and scoped Gateway AGENTS.md files were read; the relevant guidance applied because Gateway session-state PRs require whole-path review, session-state merge-risk treatment, and scoped Gateway test awareness. (AGENTS.md:1, fcb4c5d0416a)
  • Maintainer notes check: The only maintainer note present was .agents/maintainer-notes/telegram.md; no maintainer note matched the WebChat/Gateway files touched by this PR.
  • Current main failure path: Current main's async chat.send catch path records the error dedupe entry and broadcasts a chat error, then the finally cleanup removes the active run without persisting a terminal session lifecycle event. (src/gateway/server-methods/chat.ts:4936, fcb4c5d0416a)
  • Current lifecycle persistence contract: The shared lifecycle helper loads the current session row, rejects stale pre-reset lifecycle events by sessionId, derives a persisted lifecycle patch, and writes it through updateSessionStoreEntry. (src/gateway/session-lifecycle-state.ts:256, fcb4c5d0416a)
  • PR head implementation: The PR head captures a pending pre-agent dispatch lifecycle error, waits until active-run cleanup, persists an error lifecycle event when no visible active run remains, and then emits sessions.changed. (src/gateway/server-methods/chat.ts:4945, 51b45039412a)
  • PR regression coverage: The added test drives a rejected dispatchInboundMessage, waits for sessions.changed, and asserts the session is failed with hasActiveRun: false in both the event and sessions.list. (src/gateway/server.chat.gateway-server-chat.test.ts:695, 51b45039412a)

Likely related people:

  • openperf: Authored the current session lifecycle stale-event protection that this PR now reuses through persistGatewaySessionLifecycleEvent. (role: stale lifecycle guard contributor; confidence: high; commits: 613f51a7aa91; files: src/gateway/session-lifecycle-state.ts)
  • ooiuuii: Authored the merged restart shutdown lifecycle ownership work that touched session-lifecycle-state.ts, server-chat.ts, and server-methods/chat.ts. (role: adjacent restart/session lifecycle contributor; confidence: high; commits: d20fdf3b3842; files: src/gateway/session-lifecycle-state.ts, src/gateway/server-chat.ts, src/gateway/server-methods/chat.ts)
  • vincentkoc: Recent current-main history shows multiple Gateway/WebChat chat-method commits near the touched chat.send surface. (role: recent Gateway/WebChat area contributor; confidence: medium; commits: 2b75806197ab, f85ef0d1147f, cf4000b47b78; files: src/gateway/server-methods/chat.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🥚 common Tiny Signal Puff

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: purrs at green checks.
Image traits: location flaky test forest; accessory release bell; palette plum, gold, and soft gray; mood proud; pose standing beside its cracked shell; shell frosted glass shell; lighting subtle sparkle highlights; background little resolved-comment flags.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Tiny Signal Puff in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@jesse-merhi
jesse-merhi marked this pull request as ready for review May 20, 2026 07:06
Copilot AI review requested due to automatic review settings May 20, 2026 07:06
Copilot AI reviewed May 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a WebChat failure-mode where chat.send dispatch rejection could leave the persisted session lifecycle state stuck as status: "running" even though the active run was already cleaned up, causing the Control UI to remain in a running state indefinitely.

Changes:

  • Persist a terminal "error" lifecycle event on chat.send dispatch rejection so the session store updates to status: "failed" with end timestamps.
  • Broadcast a sessions.changed event on this error path with status: "failed" and hasActiveRun: false.
  • Add a regression test asserting sessions.list returns status: "failed" and hasActiveRun: false after a pre-reply dispatch rejection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/gateway/server-methods/chat.ts Persists a terminal lifecycle error and broadcasts sessions.changed when WebChat dispatch rejects before a reply.
src/gateway/server.chat.gateway-server-chat.test.ts Adds regression coverage for sessions transitioning from running → failed when dispatch rejects before reply.

Comment thread src/gateway/server-methods/chat.ts Outdated
phase: "error",
startedAt: lifecycleStartedAt,
endedAt,
error: String(err),
This was referenced May 21, 2026
Closed
Closed
@jesse-merhi
jesse-merhi force-pushed the jesse/fix-webchat-dispatch-session-status branch from 88bdd2c to 58a1c5d Compare May 23, 2026 17:16
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label May 23, 2026
@github-actions github-actions Bot mentioned this pull request May 24, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request May 29, 2026
Closed
@clawsweeper clawsweeper Bot added proof: 🎥 video Contributor real behavior proof includes video or recording evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: 🎥 video Contributor real behavior proof includes video or recording evidence. labels Jun 15, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Open
@github-actions

github-actions Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: 562f2ac5a8ca1f928dee4943bdaaf73af825db1f

@jesse-merhi
jesse-merhi force-pushed the jesse/fix-webchat-dispatch-session-status branch from 278af87 to e0e6169 Compare June 23, 2026 15:11
@github-actions github-actions Bot removed the dependencies-changed PR changes dependency-related files label Jun 23, 2026
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation channel: mattermost Channel integration: mattermost channel: nextcloud-talk Channel integration: nextcloud-talk channel: telegram Channel integration: telegram labels Jun 23, 2026
@jesse-merhi

jesse-merhi commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Merged via squash.

Thanks @jesse-merhi!

@clawsweeper clawsweeper Bot mentioned this pull request Jun 23, 2026
Open
20 tasks
@github-actions github-actions Bot mentioned this pull request Jun 23, 2026
Open
This was referenced Jun 23, 2026
Open
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

app: web-ui App: web-ui gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jesse-merhi