Skip to content

fix(codex): release timed-out app-server lanes#94685

Closed
hannesrudolph wants to merge 2 commits into
openclaw:mainfrom
hannesrudolph:codex/fix-84569-codex-lane-release
Closed

fix(codex): release timed-out app-server lanes#94685
hannesrudolph wants to merge 2 commits into
openclaw:mainfrom
hannesrudolph:codex/fix-84569-codex-lane-release

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #84569 by releasing the OpenClaw command lane when a Codex app-server attempt hits its own timeout/abort path but the app-server side does not settle promptly.

  • Wire the runner's bounded lane-release hooks into plugin-owned harness attempts, not only the native OpenClaw harness.
  • Have the Codex app-server harness signal when its turn watchdog is armed, when the watchdog timeout fires, and when explicit non-timeout abort cleanup begins.
  • Add regression coverage where a Codex attempt reports a turn/completed idle timeout and then ignores cancellation forever; a second inbound for the same session still runs after the bounded grace.

Root Cause

Confirmed: the stuck lane is a root OpenClaw/Codex harness ownership bug, not a WhatsApp delivery-only bug. Current main can time out and retire/interrupt a Codex app-server turn, but the outer runner did not give plugin-owned attempts the same lane-release contract added for native runs in PR #94082. If the app-server cleanup never settled, the command lane stayed owned and later same-session work queued indefinitely.

PR #84578 remains a separate WhatsApp final-error delivery mitigation. It does not release a stuck Codex/plugin-owned lane.

Codex contract checked directly in sibling ../codex before patching: app-server v2 exposes turn/start and turn/interrupt requests plus turn/started and turn/completed notifications; turns are terminal only once they report completed/interrupted/failed rather than inProgress.

Real behavior proof

Behavior addressed: A Codex app-server attempt can time out while waiting for turn/completed, ignore cleanup/cancellation, and leave later same-session inbounds queued forever.

Real environment tested: macOS Darwin 25.5.0 arm64, Node.js v26.3.0, pnpm v11.2.2, local OpenClaw source checkout at fdd9c31.

Exact steps or command run after this patch:

pnpm test src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts -- --reporter=verbose
pnpm test src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts extensions/codex/src/app-server/run-attempt.turn-watches.test.ts -- --reporter=verbose
pnpm format:check src/agents/embedded-agent-runner/run.ts src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts extensions/codex/src/app-server/run-attempt.ts
pnpm lint src/agents/embedded-agent-runner/run.ts src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts extensions/codex/src/app-server/run-attempt.ts
git diff --check
.agents/skills/autoreview/scripts/autoreview --mode local

Evidence after fix:

run.codex-app-server-recovery.test.ts: 1 file passed, 11 tests passed
run.compaction-loop-guard.test.ts: 1 file passed, 10 tests passed
run-attempt.turn-watches.test.ts: 1 file passed, 63 tests passed
format: all matched files use the correct format
lint: core, extensions, and scripts shards finished successfully
git diff --check: clean
autoreview: clean, no accepted/actionable findings

Observed result after fix: the new regression rejects the first ignored Codex timeout with CommandLaneTaskTimeoutError after the bounded grace, then the second run for the same session completes successfully, proving the lane drains.

What was not tested: live WhatsApp transport or a live external Codex app-server process wedge. The focused regression exercises the OpenClaw runner/queue contract and Codex plugin timeout hook path that caused the permanent same-session queue ownership.

@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Summary
The PR forwards runner lane-release callbacks into Codex app-server attempts, adds Codex app-server timeout/abort signals, and adds a same-session timeout recovery regression test.

PR surface: Source +2, Tests +57. Total +59 across 3 files.

Reproducibility: yes. source-level: current main gates lane-release callbacks away from plugin-owned harnesses while the queue releases only from those callbacks. The PR regression directly models a Codex timeout that never settles followed by a second same-session run.

Review metrics: 1 noteworthy metric.

  • Plugin Lane-Release Hooks: 3 callbacks enabled for plugin-owned attempts. The PR changes timeout-armed heartbeat, timeout release, and explicit-abort release behavior for Codex app-server attempts, which is the main merge-risk boundary.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84569
Summary: This PR is the candidate fix for the Codex/plugin-owned command-lane release part of the canonical WhatsApp/Codex stall report; WhatsApp final-error delivery and diagnostic stuck-session recovery are related but separate surfaces.

Members:

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

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

  • [P2] Rebase or rerun required checks on current main so checks-node-core-tooling is green for the mergeable head.
  • [P2] Add live or Crabbox app-server wedge proof only if maintainers want stronger confidence before accepting the late-cleanup overlap risk.

Risk before merge

  • [P1] The PR intentionally releases a same-session command lane while a timed-out Codex app-server cleanup may still be unsettled; focused source and regression coverage support the contract, but no live wedged app-server or live WhatsApp transport proof is present.
  • [P1] If a timed-out Codex app-server process emits late terminal output after the lane is released, the maintainer-visible risk is session-state or delivery overlap; the code mitigates this by clearing the timed-out binding and retiring the client, but live proof was not supplied.
  • [P1] The exact PR head is currently blocked by a stale-base required-check failure in checks-node-core-tooling; current main already updates the failing prerelease test-plan assertion, so automerge should wait for a rebase/rerun rather than merge this head as-is.

Maintainer options:

  1. Refresh Required Checks Before Automerge (recommended)
    Rebase or otherwise refresh the PR so required checks run against current main, then merge only after checks-node-core-tooling and the rest of the required set are green for the mergeable head.
  2. Require Live Wedge Proof
    Ask for a live or Crabbox Codex app-server wedge scenario if maintainers want transport-level proof that late cleanup cannot corrupt session state or delivery.
  3. Pause For Adjacent Surfaces
    Pause this PR if maintainers want to coordinate landing with the separate WhatsApp final-error delivery and diagnostic recovery reset work instead of accepting a focused Codex lane fix now.

Next step before merge

  • [P2] The remaining action is maintainer/automerge handling: refresh the stale required check run on current main and decide whether the focused proof is enough for the late-cleanup overlap risk.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes existing TypeScript timeout plumbing and focused regression coverage only.

Review details

Best possible solution:

Land this focused lane-release fix after a current-base green check run and maintainer acceptance of the late-cleanup overlap risk, while leaving WhatsApp final-error delivery to #84578.

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

Yes, source-level: current main gates lane-release callbacks away from plugin-owned harnesses while the queue releases only from those callbacks. The PR regression directly models a Codex timeout that never settles followed by a second same-session run.

Is this the best way to solve the issue?

Yes, this is the best narrow fix I found: it reuses the runner-owned bounded lane-release contract and adds Codex app-server lifecycle signals instead of adding a WhatsApp-specific workaround or recovery-layer reset. The adjacent WhatsApp payload PR and diagnostic recovery PR cover different remaining surfaces.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The PR body supplies after-fix terminal proof for focused runner and Codex app-server tests showing the stuck lane drains; live transport wedge proof is explicitly listed as not tested.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 🚀 automerge armed.

Label justifications:

  • P1: The PR targets a broken agent/channel workflow where same-session messages can remain queued behind a timed-out Codex app-server attempt.
  • merge-risk: 🚨 session-state: Merging changes when a timed-out plugin-owned Codex attempt releases same-session lane ownership while cleanup may still be unresolved.
  • merge-risk: 🚨 message-delivery: The fix affects a stall path tied to queued same-session messages and missing user replies, and live transport delivery was not proven in this review.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The PR body supplies after-fix terminal proof for focused runner and Codex app-server tests showing the stuck lane drains; live transport wedge proof is explicitly listed as not tested.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal proof for focused runner and Codex app-server tests showing the stuck lane drains; live transport wedge proof is explicitly listed as not tested.
Evidence reviewed

PR surface:

Source +2, Tests +57. Total +59 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 13 11 +2
Tests 1 58 1 +57
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 71 12 +59

What I checked:

  • Root policy read: Root AGENTS.md was read fully; its Codex dependency-source gate and agent/plugin boundary guidance apply to this review. (AGENTS.md:1, 8480ef3f86a3)
  • Scoped policy read: The scoped guides for agent tests and extensions were read; this review treats the Codex plugin as a plugin-owned runtime boundary and keeps tests focused on behavior. (extensions/AGENTS.md:1, 8480ef3f86a3)
  • Current-main gap: Current main still withholds onAttemptTimeoutArmed, onAttemptTimeout, and onAttemptAbort when pluginHarnessOwnsTransport is true, so plugin-owned Codex attempts cannot trigger the runner-owned bounded lane release. (src/agents/embedded-agent-runner/run.ts:2009, 8480ef3f86a3)
  • PR runner wiring: The PR head passes startLaneProgressHeartbeat, armAttemptTimeoutRelease, and an abort callback to all harness attempts instead of gating them away from plugin-owned transports. (src/agents/embedded-agent-runner/run.ts:2009, 517c4a48e33a)
  • Codex app-server hooks: The PR notifies the runner on the first turn-watch timeout, after the attempt idle watch is armed, and before explicit non-timeout abort cleanup interrupts the Codex turn. (extensions/codex/src/app-server/run-attempt.ts:1503, 517c4a48e33a)
  • Lane release contract: The command queue already documents that caller-owned lane timeout releases may happen while the underlying task is still unwinding, and taskTimeoutReleaseSignal rejects the active lane task with CommandLaneTaskTimeoutError. (src/process/command-queue.ts:23, 517c4a48e33a)

Likely related people:

  • vincentkoc: Merged the adjacent native lane-release contract in commit fa4f1ab and also authored current-main tooling alignment commit af3c106 that explains the stale-base check failure. (role: recent area merger and adjacent maintainer; confidence: high; commits: fa4f1abb290c, af3c10626c4b; files: src/agents/embedded-agent-runner/run.ts, src/process/command-queue.ts, src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts)
  • ajwan8998: Authored the original PR 94082 native embedded-runner lane heartbeat/release work that this PR extends to plugin-owned Codex attempts. (role: adjacent feature author; confidence: medium; commits: 29c509571ddd, fd3a5014ad7b, 0ced1d37f031; files: src/agents/embedded-agent-runner/run.ts, src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts, src/process/command-queue.ts)
  • jalehman: Recently authored merged Codex app-server attempt changes in PR 93813, which touched extensions/codex/src/app-server/run-attempt.ts near this runtime surface. (role: recent Codex app-server area contributor; confidence: medium; commits: 81dc8805b6c9, 004825a10041, 55b494610c9b; files: extensions/codex/src/app-server/run-attempt.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. P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 18, 2026
@hannesrudolph
hannesrudolph marked this pull request as ready for review June 18, 2026 21:59
This was referenced Jun 19, 2026
Open
Closed
@vincentkoc
vincentkoc force-pushed the codex/fix-84569-codex-lane-release branch from fdd9c31 to 5e5a5d6 Compare June 19, 2026 02:43
@vincentkoc
vincentkoc force-pushed the codex/fix-84569-codex-lane-release branch from 4424fe8 to 517c4a4 Compare June 19, 2026 02:58
@vincentkoc

vincentkoc commented Jun 19, 2026

Copy link
Copy Markdown
Member

Clownfish 🐠 reef update

Thanks for the work here. Clownfish got this branch swimming again without needing a replacement PR.

Source PR: #94685
Validation: pnpm test src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts; pnpm check:changed
Attribution stays exactly where it should: with the original branch and PR history.

fish notes: model gpt-5.5, reasoning medium; reviewed against 517c4a4.

@clawsweeper clawsweeper Bot added 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 Jun 19, 2026
@vincentkoc

vincentkoc commented Jun 19, 2026

Copy link
Copy Markdown
Member

/clownfish automerge

@vincentkoc vincentkoc added the clownfish:automerge Maintainer opted this Clownfish PR into bounded ClawSweeper-reviewed automerge label Jun 19, 2026
@vincentkoc

vincentkoc commented Jun 19, 2026

Copy link
Copy Markdown
Member

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.

@vincentkoc

vincentkoc commented Jun 19, 2026

Copy link
Copy Markdown
Member

/clownfish automerge

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

openclaw-clownfish Bot commented Jun 19, 2026

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 commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper saw the passing review, but the PR needs another repair pass before merge.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=517c4a48e33a17433b73106c42e137a9e492f5c1); current checks are failing: checks-node-core-tooling:FAILURE
Action: repair worker queued. Run: https://github.com/openclaw/clawsweeper/actions/runs/27809439273

I will update this PR branch, or open a safe credited replacement, if the repair worker finds a narrow CI fix.

Automerge progress:

  • 2026-06-19 06:25:25 UTC review passed 517c4a48e33a (structured ClawSweeper verdict: pass (sha=517c4a48e33a17433b73106c42e137a9e492f...)

@clawsweeper clawsweeper Bot mentioned this pull request Jun 19, 2026
Open
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the contribution here. ClawSweeper could not safely push to this branch, so it opened a replacement PR from a writable branch and carried the contributor trail along with it.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
Replacement PR: #94833
Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place.
Closing this one because the run was configured to close superseded source PRs after opening the replacement.
Attribution stays attached; the replacement just gives the fix a writable branch.
Co-author credit kept:

fish notes: reasoning high; reviewed against a2edc98.

@clawsweeper clawsweeper Bot closed this Jun 19, 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 clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge clownfish:automerge Maintainer opted this Clownfish PR into bounded ClawSweeper-reviewed automerge extensions: codex 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WhatsApp session stalls on long model_call: incomplete turn with payloads=0, reply never delivered

2 participants

@hannesrudolph @vincentkoc