fix(gateway): restart sentinel wakes the session after restart and preserves thread routing by VACInc · Pull Request #53940 · openclaw/openclaw

VACInc · 2026-03-24T20:21:07Z

Summary

Problem: restart sentinel could send a restart note without actually waking the interrupted session.
Why it matters: after restart, the in-progress session could stall instead of continuing.
What changed: startup restart-sentinel handling now queues a system event, requests an immediate wake for that session, and preserves explicit thread/topic routing through the wake-triggered reply path.
What did NOT change (scope boundary): no config changes, no heartbeat schedule changes, no broad routing rewrite outside restart-sentinel wake handling.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related fix: pass sessionKey to deliverOutboundPayloads for message:sent hook dispatch #27584
This PR fixes a bug or regression

Root Cause / Regression History (if applicable)

Root cause: the restart-sentinel path was refactored into direct outbound delivery, which no longer guaranteed an agent turn for the interrupted session.
Missing detection / guardrail: no test covered "startup consumes restart sentinel and wakes the original session."
Prior context (git blame, prior PR, issue, or refactor if known): refactor: unify outbound session context wiring plus earlier restart-sentinel routing/thread fixes.
Why this regressed now: restart note delivery survived, but session wake did not.
If unknown, what was ruled out: N/A

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/gateway/server-restart-sentinel.test.ts, src/infra/heartbeat-runner.ghost-reminder.test.ts, src/infra/outbound/targets.test.ts, src/infra/system-events.test.ts
Scenario the test should lock in: startup restart-sentinel handling wakes the original session and preserves thread/topic routing.
Why this is the smallest reliable guardrail: it covers the exact startup -> wake -> routing seam without full restart E2E.
Existing test that already covers this (if any): partial coverage only.
If no new test is added, why not: N/A

User-visible / Behavior Changes

After restart, restart sentinel now wakes the interrupted session instead of only sending a best-effort restart note.
Wake-triggered replies preserve explicit thread/topic routing.

Security Impact (required)

New permissions/capabilities? (Yes/No): No
Secrets/tokens handling changed? (Yes/No): No
New/changed network calls? (Yes/No): No
Command/tool execution surface changed? (Yes/No): No
Data access scope changed? (Yes/No): No
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: Linux
Runtime/container: Node 22, pnpm
Model/provider: N/A
Integration/channel (if any): restart sentinel + heartbeat wake routing
Relevant config (redacted): agents.defaults.heartbeat.target=last

Steps

Consume a restart sentinel on startup for a session with delivery context.
Trigger the wake path for that session.
Verify the reply routes back to the original thread/topic.

Expected

The interrupted session wakes after restart and replies in the original thread/topic.

Actual

Before: restart note could be sent without waking the session.
After: session wakes and thread/topic routing is preserved.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: targeted restart-sentinel, heartbeat-runner, outbound-target, and system-event tests pass; pnpm build passes.
Edge cases checked: retry does not drop the wake; partial turn-scoped metadata still preserves thread/topic routing.
What you did not verify: full live restart E2E against a real gateway/channel.

Review Conversations

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

Compatibility / Migration

Backward compatible? (Yes/No): Yes
Config/env changes? (Yes/No): No
Migration needed? (Yes/No): No
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert this PR
Files/config to restore: src/gateway/server-restart-sentinel.ts, src/infra/system-events.ts, src/infra/outbound/targets.ts, src/infra/heartbeat-runner.ts
Known bad symptoms reviewers should watch for: restart note appears but the session still does not continue, or replies land in the wrong thread/topic

Risks and Mitigations

Risk:
- Retried outbound restart notifications could duplicate the note in some transport failure modes.
Mitigation:
- Retry is capped, and session wake no longer depends on outbound success.

greptile-apps · 2026-03-24T20:25:06Z

Greptile Summary

This PR fixes a regression where the restart sentinel could deliver a restart note but fail to wake the interrupted session, causing the in-progress session to stall after gateway restart. The fix unconditionally calls enqueueRestartSentinelWake (which queues the system event and calls requestHeartbeatNow) before attempting outbound delivery, so the session wake no longer depends on outbound success. Thread/topic routing is preserved through the wake path via DeliveryContext stored on SystemEvent entries and propagated to resolveHeartbeatDeliveryTarget.

Key changes:

scheduleRestartSentinelWake now calls enqueueRestartSentinelWake unconditionally (before outbound delivery, and for all no-route early-return paths), replacing the old enqueueSystemEvent-on-error pattern.
Outbound delivery is retried once (750ms delay, capped at 2 attempts) with a warning log; session wake is unaffected by delivery outcome.
SystemEvent gains an optional deliveryContext field; resolveSystemEventDeliveryContext merges contexts across queued events (newest wins on conflict).
resolveHeartbeatDeliveryTarget accepts a turnSource?: DeliveryContext that overrides mutable session routing fields for the wake turn, preserving explicit thread/topic routing.
resolveSessionDeliveryTarget in heartbeat mode uses turnSourceThreadId instead of the inherited session lastThreadId to avoid routing replies into stale threads.

One minor observation: In wakeDeliveryContext construction (line 106–109), payload.deliveryContext is used as the primary argument in mergeDeliveryContext, meaning payload.deliveryContext.threadId takes priority over the top-level payload.threadId. The outbound delivery path at lines 156–160 does the opposite (payload.threadId ?? first). In the unlikely edge case where both fields are set to different values, the wake-triggered heartbeat reply and the outbound restart notice would land in different threads. See the inline comment for a suggested fix.

Confidence Score: 4/5

Safe to merge; the core fix is correct and well-tested, with only a minor theoretical edge case in threadId priority worth a follow-up.
The primary bug (session not waking after restart) is correctly fixed by making the wake unconditional before outbound delivery. The thread-routing preservation through turnSource/resolveSystemEventDeliveryContext is logically sound and validated by targeted unit and integration tests. The only finding is a P2 priority inversion in wakeDeliveryContext that could cause misrouted replies when payload.threadId and payload.deliveryContext.threadId are both set and differ — an unlikely but theoretically possible edge case. Everything else (retry logic, clone isolation, turn-source channel override, heartbeat mode threadId handling) is correct.
src/gateway/server-restart-sentinel.ts lines 106–109 — minor threadId priority inconsistency worth addressing before merge.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/server-restart-sentinel.ts
Line: 106-109

Comment:
**`payload.threadId` priority inverted vs outbound delivery path**

`mergeDeliveryContext(primary, fallback)` gives `payload.deliveryContext` priority, so if `payload.deliveryContext` already carries a `threadId`, `payload.threadId` is silently discarded. However the outbound delivery path at lines 156–160 does the opposite — `payload.threadId` wins via `??`:

```ts
const threadId =
  payload.threadId ??
  parsedTarget?.threadId ??
  sessionThreadId ??
  (origin?.threadId != null ? String(origin.threadId) : undefined);
```

In the edge case where both fields are present and differ, the wake-triggered heartbeat reply would land in a different thread than the outbound restart notice. Consider swapping the argument order to match the outbound priority:

```suggestion
  const wakeDeliveryContext = mergeDeliveryContext(
    payload.threadId != null ? { ...payload.deliveryContext, threadId: payload.threadId } : payload.deliveryContext,
    undefined,
  );
```

Or more simply:

```ts
const wakeDeliveryContext = normalizeDeliveryContext({
  ...payload.deliveryContext,
  ...(payload.threadId != null ? { threadId: payload.threadId } : {}),
});
```

This makes the priority consistent: `payload.threadId` (the top-level sentinel field) wins over `payload.deliveryContext.threadId`, matching the outbound path.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: preserve turnSource threadId in hea..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b8599b924

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d62919f08

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-24T20:56:14Z

+  const normalizedDeliveryContext = normalizeDeliveryContext(options?.deliveryContext);
  entry.lastContextKey = normalizedContextKey;
  if (entry.lastText === cleaned) {
    return false;


Include delivery context in duplicate suppression key

enqueueSystemEvent now stores deliveryContext, but the duplicate check still only compares lastText. If the same system message is queued again with a different route (for example, same restart text but a new thread/topic), the second enqueue is dropped and the old route remains in the queue; resolveSystemEventDeliveryContext will then resolve stale routing and can send the wake-triggered reply to the wrong destination. Dedup should account for normalized delivery context (or update the queued event’s context) before returning false.

Useful? React with 👍 / 👎.

I think this is directionally right as a generic system-event hardening point, but I am not taking it in this PR. The route-bearing enqueue introduced here is the restart-sentinel path, and that sentinel is consumed once on startup rather than being re-enqueued repeatedly with changing delivery context. For this branch I kept the fix scoped to the concrete wake regressions; if maintainers want, I can follow up separately on broader queue dedup semantics.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c66b621e40

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 033d9c35ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-25T02:24:10Z

+    // heartbeat mode intentionally drops inherited thread IDs to avoid replying
+    // in stale threads (e.g., Slack thread_ts). The sentinel's delivery context
+    // carries the correct topic/thread ID when present.
+    turnSourceThreadId: params.turnSource?.threadId,


Gate turn-source thread IDs to target=last heartbeats

resolveHeartbeatDeliveryTarget now always passes turnSourceThreadId even when heartbeat.target is an explicit channel (not last). In that case resolveSessionDeliveryTarget can still attach the queued event's thread ID if the explicit channel matches the session's last channel, so an explicitly configured destination (e.g., fixed Telegram/Slack chat) can be forced into an unrelated old thread/topic after a restart wake. This is a regression from prior behavior (heartbeat mode dropped inherited thread IDs) and can misroute production heartbeat replies whenever a queued system event carries threadId.

Useful? React with 👍 / 👎.

Previously the restart sentinel note was delivered as an outbound message with bestEffort:true, which silently swallowed failures. Even on success, it never triggered an agent turn so the agent couldn't act on the note. Changes: - Always enqueue system event AND request heartbeat wake for the session - Switch outbound delivery to bestEffort:false with retry (2 attempts) - Log warnings on delivery failures instead of swallowing silently - Add dedicated logger for restart-sentinel subsystem

… routing Previously heartbeat mode dropped ALL threadIds, including the restart sentinel's delivery context threadId. This meant sentinel-triggered agent turns couldn't route replies back to Telegram forum topics. Changes: - resolveSessionDeliveryTarget now preserves turnSourceThreadId in heartbeat mode (but still drops session-inherited threadIds) - resolveHeartbeatDeliveryTarget passes only explicit turnSource threadId, not merged session threadId - Added tests for turn-source thread routing in heartbeat mode

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 958155acb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-25T04:42:43Z

+    // heartbeat mode intentionally drops inherited thread IDs to avoid replying
+    // in stale threads (e.g., Slack thread_ts). The sentinel's delivery context
+    // carries the correct topic/thread ID when present.
+    turnSourceThreadId: params.turnSource?.threadId,


Skip turn-source thread IDs for non-last heartbeat targets

resolveHeartbeatDeliveryTarget now always passes params.turnSource?.threadId into resolveSessionDeliveryTarget, even when heartbeat.target is an explicit channel (not last). In that explicit-target case, if the configured channel matches the session’s lastChannel, heartbeat mode will still attach this thread/topic ID, so stale system-event routing metadata can force replies into an unrelated thread. This is a routing regression for explicit heartbeat destinations, which previously did not inherit thread IDs in heartbeat mode.

Useful? React with 👍 / 👎.

@VACInc