Skip to content

fix(gateway): ignore stale abort markers for fresh chat events#91013

Merged
clawsweeper[bot] merged 4 commits into
openclaw:mainfrom
nxmxbbd:repair/post84306-stale-abort-current-20260606T203537Z
Jun 19, 2026
Merged

fix(gateway): ignore stale abort markers for fresh chat events#91013
clawsweeper[bot] merged 4 commits into
openclaw:mainfrom
nxmxbbd:repair/post84306-stale-abort-current-20260606T203537Z

Conversation

@nxmxbbd

@nxmxbbd nxmxbbd commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

What problem does this PR solve?

  • Prevents a retained chat abort marker from suppressing chat events for a fresh run registered after that marker.
  • Keeps current-abort behavior intact: an abort marker created for the active run still suppresses live chat projection and cleanup proceeds.
  • Keeps restart-shutdown abort markers on the same structured ordering contract as normal aborts.

Why does this matter now?

  • Abort markers live longer than normal chat send dedupe entries, so a reused run/idempotency key can otherwise inherit stale abort state.

What is the intended outcome?

  • Stale same-key abort markers are ignored for fresh chat event projection.
  • Current same-millisecond abort markers still suppress the active run.
  • Restart-shutdown markers use the same { abortedAtMs, sequence } form as normal abort markers.

What is intentionally out of scope?

  • No client UI/native ack handling changes.
  • No provider/model routing changes.
  • No changes to the chat send idempotency replay contract.

What does success look like?

  • Fresh chat events emit delta and final when the only abort marker is older than the fresh registration.
  • Current same-millisecond abort markers still suppress live chat events.
  • A restart-shutdown marker does not suppress a later same-key registration.

What should reviewers focus on?

  • ChatAbortMarker freshness ordering in src/gateway/server-chat-state.ts.
  • The two abort checks in src/gateway/server-chat.ts that compare markers against the current chat run link.
  • Compatibility of maintenance/retry paths that still need marker timestamps.
  • The restart shutdown terminal-run branch in src/gateway/server-close.ts using the same marker factory.

Linked context

Which issue does this close?

Fixes #91012

Which issues, PRs, or discussions are related?

Related #84176

Was this requested by a maintainer or owner?

No direct maintainer request; this is a focused follow-up to the current Gateway abort/send behavior.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed:

Stale chat abort markers should not suppress events for a fresh chat run registered after that marker, while a current abort marker should still suppress the active run.

  • Real environment tested:

Local OpenClaw checkout on this branch, importing the production Gateway chat runtime from src/gateway/server-chat.ts with tsx and isolated config/state.

  • Exact steps or command run after this patch:

From the repository root, a standalone Node runtime harness imported src/gateway/server-chat.ts, created real createChatRunState() / createAgentEventHandler() instances, and drove two cases:

  1. stale same-millisecond older-sequence marker, then assistant delta + lifecycle end;
  2. current same-millisecond abort marker, then assistant delta + lifecycle end.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):

Terminal capture from this branch, copied live output:

{
  "stale_marker_same_millisecond_older_sequence": {
    "chat_states": [
      "delta",
      "final"
    ],
    "node_chat_states": [
      "delta",
      "final"
    ],
    "marker_retained": true,
    "registry_cleared": true
  },
  "current_same_millisecond_abort_marker": {
    "chat_states": [],
    "node_chat_states": [],
    "marker_retained": true,
    "registry_cleared": true
  }
}
proof passed: stale marker ignored; current abort marker honored
  • Observed result after fix:

The stale marker case emitted both delta and final for chat and node subscribers, retained the marker for TTL cleanup, and cleared the registry. The current marker case emitted no chat/node states, retained the marker, and cleared the registry.

  • What was not tested:

External provider/network execution and browser/native client rendering were not exercised; this change is limited to Gateway chat abort marker freshness and event projection.

  • Proof limitations or environment constraints:

The runtime proof is a local production-source harness rather than a hosted end-to-end provider run. Focused regression tests and type/lint/format gates are included below as supplemental coverage.

  • Before evidence (optional but encouraged):

Current-main RED proof for the stale marker case failed with no fresh chat states emitted:

expected [] to have a length of 2 but got +0

Tests and validation

Which commands did you run?

pnpm format:check -- <13 gateway files>
pnpm exec oxlint <13 gateway files>
pnpm tsgo:core
pnpm tsgo:core:test
pnpm test:gateway -- src/gateway/server-chat.agent-events.test.ts src/gateway/chat-abort.test.ts src/gateway/server-methods/chat.abort-persistence.test.ts src/gateway/server-close.test.ts
git diff --check upstream/main..HEAD
git merge-tree --write-tree --name-only HEAD upstream/main

What regression coverage was added or updated?

  • Added stale older-timestamp marker coverage.
  • Added stale same-millisecond older-sequence marker coverage.
  • Added current same-millisecond abort marker coverage to prove active abort suppression still works.
  • Updated marker timestamp extraction coverage for maintenance/retry compatibility.
  • Added restart-shutdown marker coverage proving the terminal-run branch writes a structured marker and a later same-key registration is fresh.

What failed before this fix, if known?

  • The new stale-marker regression failed on current main because no fresh chat states were emitted.

If no test was added, why not?

N/A --- focused regression coverage was added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Fresh chat events are no longer hidden by stale abort markers.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Cancellation/lifecycle idempotency around clientRunId and provider event runId aliases.

How is that risk mitigated?

The patch stamps chat run registrations and abort markers through one process-local ordering sequence, checks both aliases against the current chat link, keeps legacy numeric marker timestamp fallback, and preserves TTL timestamp extraction for maintenance/retry paths.

Current review state

What is the next action?

Maintainer review and normal PR CI after the branch update.

What is still waiting on author, maintainer, CI, or external proof?

Nothing known from the author side; local conflict repair and focused validation are complete.

Which bot or reviewer comments were addressed?

Existing automated review requested maintainer review; no author-side blocker remains known.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: M proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: passed. Reviewed June 19, 2026, 2:29 AM ET / 06:29 UTC.

Summary
The branch stamps Gateway chat run registrations and abort markers with ordering metadata, uses freshness checks for chat projection suppression, and updates abort/restart/maintenance tests and related types.

PR surface: Source +79, Tests +103. Total +182 across 13 files.

Reproducibility: yes. source-level: on current main, seed abortedRuns for a client run id, register a same-key fresh run, then send assistant delta and lifecycle end; the presence-only checks suppress both projections. I did not execute tests in this read-only review.

Review metrics: 1 noteworthy metric.

  • Abort Projection Checks: 2 changed. Both Gateway chat projection paths now depend on abort-marker freshness before hiding user-visible delta/final events.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #91012
Summary: This PR is the linked candidate fix for the stale abort-marker issue; the older chat.abort race report is adjacent abort work but not the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

  • none.

Risk before merge

  • [P1] Abort freshness now gates whether Gateway hides or emits chat delta/final events, so an ordering mistake could leak output from an active abort or continue hiding a fresh run.
  • [P1] The PR intentionally leaves chat.send replay presence-based for retained abort markers; maintainers should accept that scope before landing.

Maintainer options:

  1. Land Projection-Only Fix (recommended)
    Accept the message-delivery semantics now that maintainer automerge was requested and the PR preserves current-run abort suppression while fixing stale projection suppression.
  2. Broaden Replay Freshness First
    If maintainers want retained abort markers ignored for later same-key chat.send replay too, ask for that scope before merge rather than treating this PR as complete.

Next step before merge

  • No ClawSweeper repair lane is needed; the remaining action is the existing maintainer-triggered automerge/check gate for the exact head.

Security
Cleared: The diff only changes in-memory Gateway chat state/types and focused tests; it does not touch CI, dependencies, secrets, package metadata, downloads, or code-execution surfaces.

Review details

Best possible solution:

Land the ordered abort-marker freshness fix through the maintainer-authorized automerge path after exact-head checks and mergeability gates pass, keeping chat.send replay semantics unchanged unless maintainers request broader behavior.

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

Yes, source-level: on current main, seed abortedRuns for a client run id, register a same-key fresh run, then send assistant delta and lifecycle end; the presence-only checks suppress both projections. I did not execute tests in this read-only review.

Is this the best way to solve the issue?

Yes. The PR puts ordering metadata in the Gateway chat-state owner and applies the same helper at both projection checks; keeping chat.send replay unchanged is acceptable once maintainers accept that scope.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The PR body includes copied terminal output from a production-source Gateway runtime harness showing stale markers emit delta/final after the patch while current abort markers still suppress output, plus before-fix failure text.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 🚀 automerge armed.

Label justifications:

  • P2: This is a bounded Gateway chat projection bug fix with medium message-delivery impact and focused regression coverage.
  • merge-risk: 🚨 message-delivery: The diff changes the suppression decision that determines whether fresh chat delta/final events are delivered or hidden.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The PR body includes copied terminal output from a production-source Gateway runtime harness showing stale markers emit delta/final after the patch while current abort markers still suppress output, plus before-fix failure text.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output from a production-source Gateway runtime harness showing stale markers emit delta/final after the patch while current abort markers still suppress output, plus before-fix failure text.
Evidence reviewed

PR surface:

Source +79, Tests +103. Total +182 across 13 files.

View PR surface stats
Area Files Added Removed Net
Source 11 112 33 +79
Tests 2 109 6 +103
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 13 221 39 +182

What I checked:

  • Repository policy read: Read the full root AGENTS.md plus scoped Gateway policies; the review applied the root ClawSweeper/Gateway depth requirements and the Gateway hot-path guidance. (AGENTS.md:1, 5d6ac2308657)
  • Current main still has presence-only suppression: Current main still computes abort suppression from abortedRuns.has(clientRunId) or abortedRuns.has(evt.runId), so it cannot distinguish stale markers from the active run marker. (src/gateway/server-chat.ts:564, 5d6ac2308657)
  • PR adds ordered marker state: The PR adds ChatRunRegistration/ChatRunEntry freshness fields, structured ChatAbortMarker values, timestamp extraction, and isChatAbortMarkerCurrent in the Gateway chat-state owner module. (src/gateway/server-chat-state.ts:20, 6f13d6f7c25e)
  • PR updates both projection checks: Both Gateway chat projection suppression checks now call isChatAbortMarkerCurrent for the client run id and provider event run id instead of suppressing solely on marker presence. (src/gateway/server-chat.ts:568, 6f13d6f7c25e)
  • Replay and TTL callers preserve timestamp contract: chat.send replay converts structured markers back to endedAt timestamps, and Gateway maintenance prunes markers through the same timestamp helper instead of assuming numeric values. (src/gateway/server-methods/chat.ts:3178, 6f13d6f7c25e)
  • Regression coverage added: The added tests cover stale older-timestamp markers, same-millisecond older-sequence markers, and current same-millisecond abort markers across chat/node subscriber behavior. (src/gateway/server-chat.agent-events.test.ts:2781, 6f13d6f7c25e)

Likely related people:

  • vincentkoc: Current blame on the inspected abort projection lines points to Vincent Koc, recent history includes the chat abort send-race fix, and he issued the maintainer automerge command on this PR. (role: recent area contributor and automerge requester; confidence: high; commits: a619518ebed8, 6ace7a6ca8d4, dc46a67e8012; files: src/gateway/server-chat.ts, src/gateway/server-methods/chat.ts, src/gateway/server-chat-state.ts)
  • steipete: The chat abort hardening that introduced central abort-marker behavior and recurring Gateway chat changes are authored by Peter Steinberger, and shortlog shows the most history in these files. (role: introduced behavior and recurring area contributor; confidence: high; commits: a1533a17f703, 07be14c02de6, d19bc1562b07; files: src/gateway/chat-abort.ts, src/gateway/server-chat.ts, src/gateway/server-methods/chat.ts)
  • Takhoffman: Recent Gateway chat lifecycle/session metadata commits touch the same projection invariants and are useful routing context for lifecycle semantics. (role: adjacent Gateway chat lifecycle contributor; confidence: medium; commits: 87875430a88d, 320c0d65a1e6, c37e49f275fb; files: src/gateway/server-chat.ts, src/gateway/server-methods/chat.ts)
  • advaitpaliwal: Authored the partial-output-on-abort change touching the same chat.abort and chat.send surfaces, which is adjacent to abort projection behavior. (role: adjacent abort behavior contributor; confidence: medium; commits: 14fb2c05b133; files: src/gateway/chat-abort.ts, 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 proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 6, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 7, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 7, 2026
Resolve conflicts from unrelated upstream churn (control-ui first-assistant
tracing, telegram):
- server-chat-state.ts: keep upstream ChatRunTiming (incl firstAssistantEventSent)
  and chatSendTiming; name the base struct ChatRunRegistration so the derived
  ChatRunEntry freshness type still applies.
- server-methods/chat.ts: union the same-module imports (chatAbortMarkerTimestampMs
  value + ChatRunTiming type) into one statement.

No fix logic changed; re-anchors merge-base to current main.
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 9, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 9, 2026
…20260614T022510

# Conflicts:
#	src/gateway/server-chat.agent-events.test.ts
#	src/gateway/server-chat.ts
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@vincentkoc

Copy link
Copy Markdown
Member

/clownfish automerge

@openclaw-clownfish openclaw-clownfish Bot added clownfish:automerge Maintainer opted this Clownfish PR into bounded ClawSweeper-reviewed automerge clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge labels Jun 19, 2026
@openclaw-clownfish

Copy link
Copy Markdown
Contributor

Clownfish is on the reef for this PR. 🐠

I tagged clownfish:automerge and sent ClawSweeper over this exact head. If the sweep finds rough coral, failing checks, or needs-human, I will take another bounded repair lap and ask for a fresh review.

A maintainer can call /clownfish stop any time and I will drift this back to human review.

@clawsweeper clawsweeper Bot added status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 19, 2026
@clawsweeper
clawsweeper Bot merged commit cd98f19 into openclaw:main Jun 19, 2026
198 of 208 checks passed
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper merged this PR after the passing review.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=6f13d6f7c25e54ce25ee75590311fdeed18a54cf)
Merge status: merged by ClawSweeper automerge
Merged at: 2026-06-19T06:30:48Z
Merge commit: cd98f195a709

What merged:

  • The branch stamps Gateway chat run registrations and abort markers with ordering metadata, uses freshness checks for chat projection suppression, and updates abort/restart/maintenance tests and related types.
  • PR surface: Source +79, Tests +103. Total +182 across 13 files.
  • Reproducibility: yes. source-level: on current main, seed abortedRuns for a client run id, register a same-k ... end; the presence-only checks suppress both projections. I did not execute tests in this read-only review.

Automerge notes:

  • PR branch already contained follow-up commit before automerge: ci: re-trigger checks against current main
  • PR branch already contained follow-up commit before automerge: Merge upstream/main into stale-abort marker fix
  • PR branch already contained follow-up commit before automerge: Merge remote-tracking branch 'upstream/main' into nex/91013-conflict-…

The automerge loop is complete.

Automerge progress:

  • 2026-06-19 06:30:15 UTC review passed 6f13d6f7c25e (structured ClawSweeper verdict: pass (sha=6f13d6f7c25e54ce25ee75590311fdeed18a5...)
  • 2026-06-19 06:30:51 UTC merged 6f13d6f7c25e (merged by ClawSweeper automerge)

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 20, 2026
…law#91013)

Summary:
- The branch stamps Gateway chat run registrations and abort markers with ordering metadata, uses freshness checks for chat projection suppression, and updates abort/restart/maintenance tests and related types.
- PR surface: Source +79, Tests +103. Total +182 across 13 files.
- Reproducibility: yes. source-level: on current main, seed abortedRuns for a client run id, register a same-k ...  end; the presence-only checks suppress both projections. I did not execute tests in this read-only review.

Automerge notes:
- PR branch already contained follow-up commit before automerge: ci: re-trigger checks against current main
- PR branch already contained follow-up commit before automerge: Merge upstream/main into stale-abort marker fix
- PR branch already contained follow-up commit before automerge: Merge remote-tracking branch 'upstream/main' into nex/91013-conflict-…

Validation:
- ClawSweeper review passed for head 6f13d6f.
- Required merge gates passed before the squash merge.

Prepared head SHA: 6f13d6f
Review: openclaw#91013 (comment)

Co-authored-by: nxmxbbd <[email protected]>
cxbAsDev pushed a commit to cxbAsDev/openclaw that referenced this pull request Jun 23, 2026
…law#91013)

Summary:
- The branch stamps Gateway chat run registrations and abort markers with ordering metadata, uses freshness checks for chat projection suppression, and updates abort/restart/maintenance tests and related types.
- PR surface: Source +79, Tests +103. Total +182 across 13 files.
- Reproducibility: yes. source-level: on current main, seed abortedRuns for a client run id, register a same-k ...  end; the presence-only checks suppress both projections. I did not execute tests in this read-only review.

Automerge notes:
- PR branch already contained follow-up commit before automerge: ci: re-trigger checks against current main
- PR branch already contained follow-up commit before automerge: Merge upstream/main into stale-abort marker fix
- PR branch already contained follow-up commit before automerge: Merge remote-tracking branch 'upstream/main' into nex/91013-conflict-…

Validation:
- ClawSweeper review passed for head 6f13d6f.
- Required merge gates passed before the squash merge.

Prepared head SHA: 6f13d6f
Review: openclaw#91013 (comment)

Co-authored-by: nxmxbbd <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge clownfish:automerge Maintainer opted this Clownfish PR into bounded ClawSweeper-reviewed automerge gateway Gateway runtime merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: stale chat abort marker can suppress fresh chat events

2 participants