fix(wrappers): P2 "Wrapper/runner hardening" — codex retry/timeout/stale-status/budget, claude provider_status, run_duo deadline, party actor timeout, play.sh idle+lock, sonnet cold-open margin (Refs #811) - #863
Conversation
…meout/stale-status/budget, claude provider_status, run_duo deadline, party actor timeout, play.sh idle+lock, sonnet cold-open margin (Refs #811) Brings the codex/openclaw/claude lanes up to the play.sh protections per the audited P2 cluster. All changes additive + bash 3.2-clean; preserves #828's worldos_timeout shim + #841's failure-classification (both confirmed intact). F12-2 sonnet cold-open deadline 400 had thin (band-top) margin → non-opus default 400→550 in clawdnd_dm_timeout (opus unchanged 500). Updated the opus-tuning + wrapper-reliability guards; rewrote the (already-stale, routine=200) cold-open timeout proof to 360 + added opus/sonnet margin scenarios. F12-9 codex DM wrapper (play_codex_dm.sh): EXIT/INT/TERM trap now stamps provider_status "failed" on abnormal exit; codex exec wrapped in an inline worldos_timeout (WORLDOS_CODEX_TURN_TIMEOUT) + ONE session-safe retry; session budget ENFORCED via token-count spend accounting (WORLDOS_CODEX_USD_PER_MTOK) → "exhausted" stop (the budget envs were validated then never used). F12-10 claude lanes never wrote provider_status.json → factored clawdnd_write_provider_status (atomic, v1 schema) into the lib; play.sh + play_party.sh write running/stopped(turn_cap|budget|idle)/failed so the viewer buckets a dead/stopped session as no_provider instead of "unknown". F12-11 run_duo DM turn was unbounded + swallowed the real cause → wrap in worldos_timeout(clawdnd_dm_timeout); report on rc!=0 (dedup-guarded); replace the inline cold-open remint with the shared clawdnd_dm_remint_session_on_retry; keep empty-output retry as 2nd trigger. F12-12 play_party companion (actor) turn was unbounded → worldos_timeout ${WORLDOS_ACTOR_TIMEOUT:-120}; empty on failure (companion_moves skip-safe). F12-13 play.sh had no idle ceiling and no launch lock → port play_party's MAX_IDLE block + acquire/release the single-flight launch lock. Skipped as already-done (confirmed on main): F12-1/3/4/5/8 (#828), SYN-01 (#841). Tests: fast_gate Tier-0 191 passed; codex wrapper 34 (incl 6 new F12-9, 3 runtime fake-codex); wrapper-reliability/opus/adversarial 43; new shell tests test_play_party_actor_timeout.sh, test_run_duo_dm_timeout.sh, test_provider_status_claude_lanes.sh + extended single-flight + cold-open proof. Source: docs/audits/ENGINE-AUDIT-2026-06-11.md
|
Warning Review limit reached
More reviews will be available in 40 minutes and 22 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d428fafcd
ℹ️ 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".
| if [ "$status" -ne 0 ]; then | ||
| echo "[codex-dm-provider] codex turn rc=$status — retrying once (codex turns are stateless, so the retry is session-safe)" >&2 | ||
| : > "$LAST_MESSAGE" | ||
| status=0 | ||
| _codex_exec_once || status=$? |
There was a problem hiding this comment.
When codex exec times out or exits nonzero after it has already invoked engine MCP tools, this retry replays the same prompt from scratch. The non-hero opening prompt can run start_world/seating again, and a move prompt can re-apply the same player action, so a transient timeout after a successful mutation can mint duplicate campaigns or double-apply attacks/persistence. Unlike the Claude cold-open retry path, this has no live-campaign re-grounding or idempotency check before retrying.
Useful? React with 👍 / 👎.
| worldos_timeout "${WORLDOS_ACTOR_TIMEOUT:-${CLAWDND_ACTOR_TIMEOUT:-120}}" \ | ||
| claude -p "$msg" "${resume[@]}" --mcp-config "$cfg" --strict-mcp-config \ | ||
| --model "$CLAWDND_ACTOR_MODEL" --permission-mode bypassPermissions --max-budget-usd "$BUDGET" \ | ||
| --output-format stream-json --verbose > "$out" 2>> "$STATE_DIR/companion.err" || true |
There was a problem hiding this comment.
When the companion claude call times out or otherwise fails here, || true makes turn actor return like an ordinary empty/no-op turn, so actor_move immediately sends its existing “You didn't act” nudge and waits for another full WORLDOS_ACTOR_TIMEOUT. In the wedged-companion case this doubles the pre-DM stall per companion before the human move can resolve, contrary to the intended graceful skip; propagate a timeout/failure sentinel so actor_move can skip the nudge on failed attempts.
Useful? React with 👍 / 👎.
1 participant
P2 cluster "Wrapper/runner hardening" (issue #811)
Brings the codex / openclaw / claude DM lanes up to the
scripts/play.shprotections, per the audited P2 cluster. Every change is additive and bash 3.2-clean; it preserves #828'sworldos_timeoutshim + retry/timeout behavior and #841's failure-classification (both confirmed intact on main and re-used, not re-implemented).Source of truth:
docs/audits/ENGINE-AUDIT-2026-06-11.md(Part C per-finding specs) + the #811 cluster body.Fixed (this PR)
400s— equal to its own documented~280–400sband TOP (thin margin). Non-opus default400 → 550inclawdnd_dm_timeout(opus unchanged at500; the shipped default-opus path is byte-identical). Env override (WORLDOS_COLDOPEN_TIMEOUT) still wins.scripts/play_codex_dm.sh: (a) EXIT/INT/TERM trap now stampsprovider_status "failed"on an abnormal exit (was stuck"running"— viewer kept showing a live-looking dead dashboard); (b)codex execwrapped in an inlineworldos_timeout(WORLDOS_CODEX_TURN_TIMEOUT, default 500s) + ONE retry (codex turns are stateless → session-safe); (c) the budget envs (validated then never used) are now enforced —codex_session_spend_usdaccounts spend from codex's cumulativetoken_countevents at an explicit, overridable rate (WORLDOS_CODEX_USD_PER_MTOK) and stops the session"exhausted"atCLAWDND_PLAY_SESSION_BUDGET.provider_status.json→ factoredclawdnd_write_provider_status(atomic tmp+fsync+replace,worldos.provider-status.v1) intoqa/lib_beat_driver.sh;play.sh+play_party.shnow writerunning/stopped(turn_cap|budget|idle) /failed, so the viewer buckets a dead/stopped session asno_providerinstead of falling back to"unknown".qa/run_duo.shDM turn was unbounded and swallowed the real failure cause → wrapped inworldos_timeout "$(clawdnd_dm_timeout "$first")"; reports the structured cause onrc!=0with no error-class result (dedup-guarded againstclawdnd_dm_final_text); the inline cold-open remint is replaced with the sharedclawdnd_dm_remint_session_on_retry; the empty-output retry is kept as the second trigger.play_party.shcompanion (actor) turn was an unboundedclaude -p→ wrapped inworldos_timeout "${WORLDOS_ACTOR_TIMEOUT:-120}"; empty on failure, socompanion_moves'[ -n "$cm" ] &&guard skips it (graceful degradation — the beat still reaches the DM).play.sh(the.app's default solo entry point) had no idle ceiling and no launch lock → portedplay_party.sh'sMAX_IDLEblock (CLAWDND_PLAY_MAX_IDLE, default 1800s) + acquires/releases the single-flight launch lock.Skipped — already done on main (confirmed still intact)
timeout(1)shim). Re-used, not re-implemented.run_duorc-report cooperates with it (dedup-guarded).Deferred (out of this PR's scope)
play_codex_dm.shcarries an inline copy ofworldos_timeout(byte-identical behavior to the lib's canonical copy) because the codex wrapper is deliberately self-contained (ownchatlog/log_engine_narration/record_dm_replythat would 3-way clash onsource). Flagged in-code; F12-20 tracks the dedup.Tests
All green locally:
servers/engine/tests/test_codex_provider_wrapper.py): 34 (28 original + 6 new F12-9, incl. 3 runtime fake-codex: crash→failed, retry-recovers, over-budget→exhausted).qa/test_play_party_actor_timeout.sh,qa/test_run_duo_dm_timeout.sh,qa/test_provider_status_claude_lanes.sh; extendedqa/test_play_party_single_flight.sh(+play.sh lock/idle); rewrote the staleqa/dryrun_coldopen_timeout_proof.sh(it still asserted routine=200 after F12-1 moved it to 360 — it was red and not gated)./bin/bash -nclean on every touched script. Full 2351-test engine suite deferred to GitHub CI per the test-execution policy.DO NOT MERGE — for review.
Refs #811(the cluster remainder beyond F12-2/9/10/11/12/13 is not in this PR's scope).