Skip to content

fix: preserve messages when Codex steering lacks transcript acknowledgement#103916

Merged
jalehman merged 1 commit into
mainfrom
josh/clawdbot-d02-1-9-1-40-367-steering-followup
Jul 11, 2026
Merged

fix: preserve messages when Codex steering lacks transcript acknowledgement#103916
jalehman merged 1 commit into
mainfrom
josh/clawdbot-d02-1-9-1-40-367-steering-followup

Conversation

@jalehman

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where a message arriving during an active Codex app-server turn could receive no response when Telegram required transcript-backed adoption and the runtime could not wait for a transcript commit.

Why This Change Was Made

When steering fails specifically with transcript_commit_wait_unsupported, OpenClaw keeps the inbound message in the follow-up queue until the active run finishes. Other steering rejection reasons retain their existing admission behavior.

User Impact

Messages are preserved and run after the active turn when Codex cannot provide the transcript acknowledgement required for safe same-turn steering.

Evidence

  • Reproduced the bug with a regression test that returned a competing run payload instead of queueing the inbound message.
  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts
  • Result: 91 tests passed.
  • Autoreview reported no accepted or actionable findings.

Related: #98236

@openclaw-barnacle openclaw-barnacle Bot added size: XS maintainer Maintainer-authored PR labels Jul 10, 2026
@jalehman
jalehman marked this pull request as ready for review July 10, 2026 19:50
@jalehman jalehman self-assigned this Jul 10, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 10, 2026
@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 4:07 PM ET / 20:07 UTC.

Summary
The PR queues an inbound message as a follow-up when Codex rejects transcript-backed steering because transcript commit waiting is unsupported, and adds focused regression coverage.

PR surface: Source +4, Tests +29. Total +33 across 2 files.

Reproducibility: yes. at source level and in the targeted regression setup: current main can reject transcript-backed steering and continue without deferring the durable inbound message. No live Telegram/Codex artifact demonstrates the current or fixed behavior.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted live Telegram/Codex evidence showing exactly-once deferred adoption and reply delivery.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides only mocked end-to-end regression coverage; add redacted live Telegram/Codex logs, output, or a recording, update the PR body, and if review does not refresh ask a maintainer to comment @clawsweeper re-review.

Mantis proof suggestion
A live Telegram transcript would materially prove the user-visible delivery outcome that the mocked test cannot. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: reproduce a message arriving during an active Codex turn and capture a redacted transcript proving it is adopted and answered exactly once as a follow-up when transcript commit waiting is unsupported.

Risk before merge

  • [P1] Without live Telegram/Codex proof, a delivery or lifecycle mismatch could still strand, duplicate, or mis-associate the deferred message despite the source-level invariants and mocked regression coverage.

Maintainer options:

  1. Prove the real fallback (recommended)
    Capture a redacted Telegram/Codex run showing exactly one deferred adoption and reply after the active turn.
  2. Accept source-level confidence
    A maintainer can intentionally accept the test-only evidence after reviewing the delivery and lifecycle contracts.

Next step before merge

  • [P1] The only remaining blocker is the protected PR’s human proof decision; no concrete mechanical code repair is established for an automated worker.

Maintainer decision needed

  • Question: Must this protected PR provide live Telegram/Codex exactly-once follow-up proof before merge, or should a maintainer explicitly override the external-contributor proof gate?
  • Rationale: Source, history, tests, and the direct Codex contract support the implementation, but repository policy requires after-fix real behavior proof for external non-docs PRs and the current evidence is mock-only.
  • Likely owner: vincentkoc — He owns the most recent Telegram queued-adoption lifecycle on which the requested proof depends.
  • Options:
    • Require live proof (recommended): Wait for redacted logs, live output, or a recording showing the message is deferred, adopted, and replied to exactly once.
    • Override proof gate: A maintainer explicitly accepts the focused regression test and source-contract inspection without a live channel run.

Security
Cleared: The focused runtime and test diff introduces no dependency, permission, secret, artifact-download, or supply-chain concern.

Review details

Best possible solution:

Land the narrow queued fallback after real Telegram/Codex proof confirms exactly-once deferred delivery, while keeping same-turn transcript-backed steering disabled until Codex exposes durable idempotent acceptance.

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

Yes at source level and in the targeted regression setup: current main can reject transcript-backed steering and continue without deferring the durable inbound message. No live Telegram/Codex artifact demonstrates the current or fixed behavior.

Is this the best way to solve the issue?

Yes. The patch uses the established queued lifecycle and is safer than best-effort steering because current Codex appends each accepted steer and does not deduplicate clientUserMessageId retries.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The bug can suppress a durable Telegram message during a real active Codex turn.
  • add merge-risk: 🚨 message-delivery: The changed admission branch determines whether a rejected steer is preserved for later delivery.
  • add merge-risk: 🚨 session-state: Correctness relies on transferring ownership to the queued lifecycle and delaying durable adoption until later reply-lane admission.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides only mocked end-to-end regression coverage; add redacted live Telegram/Codex logs, output, or a recording, update the PR body, and if review does not refresh ask a maintainer to comment @clawsweeper re-review.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The changed behavior is directly demonstrable as a Telegram follow-up reply instead of a lost message.

Label justifications:

  • P1: The bug can suppress a durable Telegram message during a real active Codex turn.
  • merge-risk: 🚨 message-delivery: The changed admission branch determines whether a rejected steer is preserved for later delivery.
  • merge-risk: 🚨 session-state: Correctness relies on transferring ownership to the queued lifecycle and delaying durable adoption until later reply-lane admission.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides only mocked end-to-end regression coverage; add redacted live Telegram/Codex logs, output, or a recording, update the PR body, and if review does not refresh ask a maintainer to comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The changed behavior is directly demonstrable as a Telegram follow-up reply instead of a lost message.
Evidence reviewed

PR surface:

Source +4, Tests +29. Total +33 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 1 +4
Tests 1 32 3 +29
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 37 4 +33

What I checked:

  • Current-main failure path: Current main requests transcript commit waiting when durable adoption is required; after a rejected steer, steer-mode admission can continue without forcing the inbound message into the follow-up queue. (src/auto-reply/reply/agent-runner.ts:1286, 1bcc4c5e7063)
  • Proposed fallback is narrowly scoped: The PR forces follow-up admission only for transcript_commit_wait_unsupported; other steering rejection reasons retain their existing behavior. (src/auto-reply/reply/agent-runner.ts:1320, 348182ab12a8)
  • Telegram deferred-adoption lifecycle: Telegram marks a queued turn deferred at enqueue, performs durable adoption when the queued turn is admitted, and abandons it if queue ownership completes without admission. (extensions/telegram/src/bot-message-dispatch.ts:2651, 1bcc4c5e7063)
  • Follow-up admission boundary: The follow-up runner calls the queued lifecycle admission hook only after acquiring reply-lane ownership, so durable ingress adoption precedes later-turn execution. (src/auto-reply/reply/followup-runner.ts:720, 1bcc4c5e7063)
  • Codex dependency contract: Codex appends every accepted steer as a new pending UserInput; clientUserMessageId is carried as correlation metadata and is not checked as an idempotency key. (../codex/codex-rs/core/src/session/mod.rs:3931, a328f30172c6)
  • Upstream capability remains missing: The open Codex issue requests durable retry idempotency for turn/steer, confirming that OpenClaw cannot safely treat current RPC acceptance as authoritative transcript adoption.

Likely related people:

  • obviyus: Ayaan Zaidi introduced the channel turn-adoption acknowledgement and recently maintained active-run steering gates. (role: introduced behavior; confidence: high; commits: ef103205adab, c2789b52a7fd; files: src/auto-reply/reply/agent-runner.ts, src/agents/embedded-agent-runner/runs.ts)
  • vincentkoc: Vincent Koc authored and merged the current Telegram deferred, admitted, and abandoned spooled-turn lifecycle that this fallback uses. (role: recent lifecycle owner; confidence: high; commits: f79e8859855b; files: extensions/telegram/src/bot-message-dispatch.ts, src/auto-reply/reply/queue/types.ts, src/auto-reply/reply/agent-runner.ts)
  • steipete: Peter Steinberger recently maintained admitted follow-up ownership and restart behavior on current main, which shares the deferred-work invariant. (role: adjacent recent contributor; confidence: medium; commits: 1bcc4c5e7063; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/followup-runner.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.

@jalehman

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis telegram live: reproduce a message arriving during an active Codex turn and capture a redacted transcript proving it is adopted and answered exactly once as a follow-up when transcript commit waiting is unsupported.

@jalehman
jalehman merged commit f68af64 into main Jul 11, 2026
203 of 217 checks passed
@jalehman
jalehman deleted the josh/clawdbot-d02-1-9-1-40-367-steering-followup branch July 11, 2026 00:34
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 11, 2026
vincentkoc pushed a commit that referenced this pull request Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant