Skip to content

Recover stale pending final deliveries#90855

Open
jeremykraklist wants to merge 2 commits into
openclaw:mainfrom
jeremykraklist:son-of-anton/pending-final-delivery-recovery-20260605
Open

Recover stale pending final deliveries#90855
jeremykraklist wants to merge 2 commits into
openclaw:mainfrom
jeremykraklist:son-of-anton/pending-final-delivery-recovery-20260605

Conversation

@jeremykraklist

@jeremykraklist jeremykraklist commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • replay durable pending final-delivery text only when the saved delivery context exactly matches the current source-reply route
  • mark pending final-delivery state in diagnostics and classify queued stale work with pending final delivery as recoverable
  • auto-enable lightweight heartbeat bootstrap context for already-huge sessions to avoid context spiral, while preserving explicit lightContext: false

Real behavior proof

  • Behavior or issue addressed: Slack/source-channel sessions that generated a final reply but stalled with pendingFinalDelivery=true can now safely replay that pending final on the same route instead of staying stuck; huge heartbeat sessions now switch to lightweight bootstrap context to avoid another compaction/context spiral.
  • Real environment tested: MBA HQ local OpenClaw checkout/worktree at exact head 4ff5e4d2cb24f936f84fd868be70e4c53e2629ea; before-state was confirmed read-only from the Contabo/OpenClaw production session/log store for the June 5 incident.
  • Exact steps or command run after this patch: git rev-parse HEAD; git diff --check origin/main...HEAD; node scripts/test-projects.mjs src/auto-reply/reply/pending-final-delivery.test.ts src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts src/logging/diagnostic-session-attention.test.ts src/infra/heartbeat-runner.model-override.test.ts -- --reporter=verbose; pnpm check:test-types; pnpm build.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Copied terminal output from the exact-head local OpenClaw setup: 
    $ git rev-parse HEAD
4ff5e4d2cb24f936f84fd868be70e4c53e2629ea

$ git diff --check origin/main...HEAD
# exit 0

$ node scripts/test-projects.mjs src/auto-reply/reply/pending-final-delivery.test.ts src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts src/logging/diagnostic-session-attention.test.ts src/infra/heartbeat-runner.model-override.test.ts -- --reporter=verbose
[test] passed 3 Vitest shards in 16.26s

$ pnpm check:test-types
$ pnpm tsgo:test
$ pnpm tsgo:core:test && pnpm tsgo:extensions:test
# exit 0

$ pnpm build
[build-all] phase timings: total 907.9s; slowest tsdown 885.9s; write-cli-startup-metadata 11.7s; build:plugin-sdk:dts (cached) 2.43s; ui:build 2.02s; plugins:assets:build 1.75s; runtime-postbuild 1.52s; plugins:assets:copy 581ms; check-cli-bootstrap-imports 465ms; write-plugin-sdk-entry-dts (cached) 461ms; check-plugin-sdk-exports 408ms; write-cli-compat 142ms; write-build-info 130ms; copy-hook-metadata 118ms; build-stamp 106ms; runtime-postbuild-stamp 104ms; copy-export-html-templates (cached) 8ms
# exit 0
  • Observed result after fix: Exact-head local validation passed. Focused replay/route-safety, diagnostics, heartbeat light-context coverage, and the full dispatch shard passed; test typecheck passed after adding the session-store fixture token fields; full build completed successfully before the final test-only mock export patch.
  • What was not tested: No production deployment, gateway restart, live Slack delivery replay, live session-store mutation, or live heartbeat run was performed.
  • Proof limitations or environment constraints: This PR intentionally stops at code + local exact-head validation. The Contabo evidence was gathered read-only; applying the behavior to the live incident would require a separate approved deploy/restart path.
  • Before evidence (optional but encouraged): Read-only Contabo pass showed agent:main:slack:direct:u039p2yjr29 still had pendingFinalDelivery=true after generated final output, while the mc-gateway...:main lane had an estimated 2,530,988 prompt tokens against a 1,000,000 budget and prior context_length_exceeded during compaction.

Tests and validation

Which commands did you run?

  • git diff --check origin/main...HEAD
  • node scripts/test-projects.mjs src/auto-reply/reply/pending-final-delivery.test.ts src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts src/logging/diagnostic-session-attention.test.ts src/infra/heartbeat-runner.model-override.test.ts -- --reporter=verbose
  • pnpm check:test-types
  • pnpm build

What regression coverage was added or updated?

  • Added same-route pending-final replay checks, route mismatch non-replay coverage, pending final clearing coverage, diagnostic attention classification coverage, and huge heartbeat session lightweight-context coverage.

What failed before this fix, if known?

  • CI check-test-types initially failed because the shared heartbeat test fixture type did not include the newly seeded totalTokens fields. Fixed in this head.
  • Real behavior proof initially failed because this external fork PR body did not include the required proof section. Added here.

If no test was added, why not?

  • Tests were added/updated.

Risk checklist

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

Yes. A stalled pending final can now be replayed to the exact same route before new model work.

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

No config/env/migration shape change. Heartbeat runtime now auto-selects lightweight bootstrap context for huge sessions unless explicitly disabled.

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

No auth/secrets/tool execution change. Network/send behavior changes only for same-route pending final replay after existing send-policy and suppression checks.

What is the highest-risk area?

Duplicate or wrong-route final delivery.

How is that risk mitigated?

Replay is gated by exact channel/target/account/thread context matching, send-policy/suppression checks, sanitized non-empty text, and pending state is cleared only after a successful final dispatch.

@openclaw-barnacle openclaw-barnacle Bot added size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 1:10 PM ET / 17:10 UTC.

Summary
The branch adds same-route cached pending-final replay, Slack DM target normalization, pending-final diagnostics, automatic lightweight heartbeat context for huge sessions, and focused tests.

PR surface: Source +330, Tests +528. Total +858 across 18 files.

Reproducibility: yes. source-level. The PR head rewrites Slack user-scoped pending contexts when current user proof is absent and sends cached pending-final text after route matching without a created-at or attempt-count gate.

Review metrics: 2 noteworthy metrics.

  • Heartbeat Context Default: 1 automatic default changed. Huge heartbeat sessions now switch to lightweight bootstrap context unless explicitly disabled, which is upgrade-sensitive behavior.
  • Current-Head Proof Runs: 0 proof runs at e85a541. The PR body proof was captured at an earlier head, so it does not prove the live production-code diff.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Resolve the conflicting / DIRTY merge state against current main.
  • [P1] Fix the Slack user-proof and cached replay-bound findings.
  • Refresh terminal, log, or live proof at the current head with private details redacted.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes copied terminal output for 4ff5e4d, but the live head is e85a541 with later production changes and no current-head or live Slack/gateway proof. 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.

Proof path suggestion
A real Slack smoke would materially help prove that pending-final replay delivers once to the correct DM and does not resurrect stale or mismatched finals. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] GitHub reports the branch as CONFLICTING / DIRTY, so the current diff is not a trustworthy merge result.
  • [P1] Same-route replay can deliver cached pending-final text without checking age, attempt count, or an abandonment policy.
  • [P1] Slack DM normalization can convert a saved user-scoped target to the current native DM when same-user proof is absent, risking wrong-recipient private final delivery.
  • [P1] Automatic lightweight heartbeat context changes the default behavior for existing huge sessions unless maintainers deliberately accept that upgrade behavior.
  • [P1] The supplied terminal proof is for an earlier head and does not prove the current e85a541 branch behavior or a live Slack/gateway replay path.

Maintainer options:

  1. Repair Replay Gates Before Merge (recommended)
    Require same-user Slack proof and add an explicit age or attempt bound before any cached pending-final payload is delivered.
  2. Accept Recovery Policy Deliberately
    Maintainers can choose broader replay and automatic light-context behavior, but that choice should be explicit and backed by current-head proof.
  3. Pause For Rebase Or Replacement
    Because the branch is currently dirty, maintainers can pause this PR and use a narrower current-main-compatible replacement instead.

Next step before merge

  • [P1] Manual review is needed because the branch is conflicting and the remaining blockers require contributor proof plus maintainer replay/heartbeat policy decisions, not a safe automated repair.

Security
Needs attention: The diff has a concrete privacy-sensitive routing concern in Slack DM target rewriting; no supply-chain or dependency concerns were found.

Review findings

  • [P1] Require user proof before rewriting Slack DM targets — src/auto-reply/reply/pending-final-delivery.ts:103-107
  • [P1] Bound cached pending-final replay before delivery — src/auto-reply/reply/dispatch-from-config.ts:2154
Review details

Best possible solution:

Rebase onto current main, require matched Slack user proof before DM target rewriting, add a freshness or attempt gate before cached replay, keep heartbeat light-context default behavior explicit, and refresh current-head real behavior proof.

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

Yes, source-level. The PR head rewrites Slack user-scoped pending contexts when current user proof is absent and sends cached pending-final text after route matching without a created-at or attempt-count gate.

Is this the best way to solve the issue?

No, not as submitted. Same-route replay is a plausible mitigation, but the best fix needs stricter route proof, stale-state bounds, conflict resolution, and refreshed current-head proof.

Full review comments:

  • [P1] Require user proof before rewriting Slack DM targets — src/auto-reply/reply/pending-final-delivery.ts:103-107
    When directUserTarget is absent or not a user:U... value, expectedUser is undefined and this helper still rewrites a saved user:U... pending target to the provided native DM channel. That can make a stale pending final look same-route for a different DM; keep the user-scoped context unless the current or stored user proof matches.
    Confidence: 0.86
  • [P1] Bound cached pending-final replay before delivery — src/auto-reply/reply/dispatch-from-config.ts:2154
    This sends the cached pending-final payload whenever the route matches, without checking pending age, attempt count, or a stale-state abandonment policy. The linked stale replay reports show old pending finals can become obsolete user-visible messages, so add an explicit freshness or attempt gate before delivery.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 security-boundary: The Slack DM rewrite can send cached private final text to a native DM without proving it belongs to the same Slack user.
  • remove merge-risk: 🚨 session-state: Current PR review merge-risk labels are merge-risk: 🚨 compatibility, merge-risk: 🚨 message-delivery, merge-risk: 🚨 security-boundary.

Label justifications:

  • P1: The PR affects a real user-facing agent/channel workflow where final replies can stay stuck, duplicate, or be delivered to the wrong destination.
  • merge-risk: 🚨 compatibility: The heartbeat change automatically changes default context behavior for existing huge sessions unless users explicitly set lightContext false.
  • merge-risk: 🚨 message-delivery: The diff changes when cached final replies are replayed and cleared, which can duplicate, suppress, or misroute visible messages.
  • merge-risk: 🚨 security-boundary: The Slack DM rewrite can send cached private final text to a native DM without proving it belongs to the same Slack user.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes copied terminal output for 4ff5e4d, but the live head is e85a541 with later production changes and no current-head or live Slack/gateway proof. 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

PR surface:

Source +330, Tests +528. Total +858 across 18 files.

View PR surface stats
Area Files Added Removed Net
Source 11 359 29 +330
Tests 7 531 3 +528
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 18 890 32 +858

Security concerns:

  • [medium] Slack DM rewrite can misroute private finals — src/auto-reply/reply/pending-final-delivery.ts:104
    resolveSlackDirectPendingFinalDeliveryContext rewrites a saved user-scoped Slack pending target to the provided native DM when expectedUser is absent, so a current DM can make older user-scoped text look same-route without proving the same user.
    Confidence: 0.78

What I checked:

  • Root and scoped policy read: Root AGENTS.md and src/agents/AGENTS.md were read; the review applied OpenClaw guidance for compatibility-sensitive session state, message delivery, and agent recovery changes. (AGENTS.md:1, f284ce3b4df7)
  • Live PR state: GitHub reports head e85a541 as CONFLICTING / DIRTY, so the displayed patch is not a reliable merge result. (e85a54109e27)
  • Current main does not already implement the PR surface: Current main has no same-route pending-final replay helper, Slack direct pending-final context helper, diagnostic pending-final marker, or automatic heartbeat token-threshold light-context constant. (f284ce3b4df7)
  • PR sends cached final without a stale-state gate: The PR head sends sanitized cached pending-final text after route matching, but the replay block has no age, attempt-count, or abandonment check before delivery. (src/auto-reply/reply/dispatch-from-config.ts:2154, e85a54109e27)
  • Slack DM rewrite lacks same-user proof: When expectedUser is absent, the PR helper rewrites a saved user-scoped Slack target to the provided native DM channel instead of requiring proof that both refer to the same user. (src/auto-reply/reply/pending-final-delivery.ts:103, e85a54109e27)
  • Current-main heartbeat invariant: Current main tests preserve non-ack pending-final text without direct heartbeat replay and leave attempt count undefined, so a fix must not reintroduce unbounded cached final delivery. (src/auto-reply/reply/get-reply.fast-path.test.ts:300, f284ce3b4df7)

Likely related people:

  • MertBasar0: Authored the merged durable main-session pending-final delivery machinery that the PR modifies across reply, dispatch, and heartbeat paths. (role: introduced behavior; confidence: high; commits: c240e718e91e; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/get-reply.ts)
  • agocs: Authored the recent heartbeat pendingFinalDelivery clear-on-success and ownership-gated cleanup work that defines adjacent current-main behavior. (role: recent area contributor; confidence: high; commits: 305fa9c4ddbe; files: src/infra/heartbeat-runner.ts, src/infra/heartbeat-runner.clears-pending-final-delivery.test.ts)
  • kesslerio: Authored the merged heartbeat no-direct-replay invariant that this PR changes around by adding same-route cached final replay. (role: recent area contributor; confidence: high; commits: d00e764e6655; files: src/auto-reply/reply/get-reply.ts, src/auto-reply/reply/get-reply.fast-path.test.ts, src/infra/heartbeat-runner.ts)
  • jalehman: Recent restart-recovery lifecycle and transcript-reader seam work touches the owner boundary for main-session recovery behavior. (role: adjacent owner; confidence: medium; commits: fd8e3bf65201, d216f7c876dd; files: src/agents/main-session-restart-recovery.ts)
  • vincentkoc: Recent dispatch/session integration commits touch the current main dispatch path and fast auto delivery semantics adjacent to pending-final cleanup. (role: recent area contributor; confidence: medium; commits: 9e8ab083dd6b; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/get-reply.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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T17:35:27.940Z sha e85a541 :: needs real behavior proof before merge. :: [P1] Require user proof before rewriting Slack DM targets | [P1] Bound cached pending-final replay before delivery

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 6, 2026
@jeremykraklist
jeremykraklist force-pushed the son-of-anton/pending-final-delivery-recovery-20260605 branch from 53175f7 to 2f36319 Compare June 6, 2026 05:14
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 6, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@jeremykraklist
jeremykraklist force-pushed the son-of-anton/pending-final-delivery-recovery-20260605 branch from 2f36319 to b1e6a3d Compare June 6, 2026 05:34
@jeremykraklist
jeremykraklist force-pushed the son-of-anton/pending-final-delivery-recovery-20260605 branch from b1e6a3d to 4ff5e4d Compare June 6, 2026 05:36
@openclaw-barnacle openclaw-barnacle Bot added the agents Agent runtime and tooling label Jun 6, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 6, 2026
This was referenced Jun 19, 2026
Closed
Open
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 21, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Closed
@clawsweeper clawsweeper Bot added merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 6, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 21, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: L stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jeremykraklist