Skip to content

fix(codex): preserve source reply mode for active runs#86287

Closed
ferminquant wants to merge 1 commit into
openclaw:mainfrom
ferminquant:codex/issue-86232-source-reply-mode
Closed

fix(codex): preserve source reply mode for active runs#86287
ferminquant wants to merge 1 commit into
openclaw:mainfrom
ferminquant:codex/issue-86232-source-reply-mode

Conversation

@ferminquant

@ferminquant ferminquant commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

AI assistance disclosure

This PR was prepared with AI assistance. I inspected the reported issue, related OpenClaw threads, source paths, queue contract, and validation output before making the change.

Real behavior proof

Behavior addressed: Codex app-server active runs now carry message_tool_only source reply delivery mode into the shared active-run queue, allowing sub-agent completion wakeups to steer the active Codex turn instead of being rejected as source_reply_delivery_mode_mismatch.

Real environment tested: Local OpenClaw source runtime in a dedicated worktree, using the production queueAgentHarnessMessage active-run guard and the Codex app-server steering queue.

Exact steps or command run after this patch:

timeout 60s node --import tsx .artifacts/issue-86232-codex-source-reply-proof.mjs

Evidence after fix:

{
  "proof": "issue-86232-codex-source-reply-delivery-mode",
  "mismatchedMode": {
    "queued": false,
    "requestMethods": []
  },
  "matchedMode": {
    "sourceReplyDeliveryModeOnHandle": "message_tool_only",
    "queued": true,
    "steerMethod": "turn/steer",
    "steerParams": {
      "threadId": "proof-thread-86232",
      "expectedTurnId": "proof-turn-86232",
      "input": [
        {
          "type": "text",
          "text": "subagent complete",
          "text_elements": []
        }
      ]
    },
    "requestMethods": [
      "turn/steer"
    ]
  },
  "expected": {
    "mismatchedQueued": false,
    "matchedQueued": true,
    "matchedMethod": "turn/steer"
  }
}

Observed result after fix: A handle without sourceReplyDeliveryMode still rejects a message-tool-only source reply, while a Codex-style active handle with sourceReplyDeliveryMode: "message_tool_only" accepts the same reply and emits turn/steer with the sub-agent completion text.

What was not tested: No live Discord group chat was used, so this proof does not verify Discord transport credentials, gateway delivery, or final message-tool posting. It proves the source runtime contract that caused the reported silent rejection.

Root Cause

The shared active-run queue rejects source replies marked message_tool_only unless the registered active run handle also advertises sourceReplyDeliveryMode: "message_tool_only". The regular PI embedded runner already passes that field into its active handle, but the Codex app-server runner omitted it when registering its handle with setActiveEmbeddedRun. As a result, Codex app-server group sessions that required message-tool-only delivery could reject sub-agent completion wakeups before steering the active turn.

Regression Test Plan

  • The new test starts the Codex app-server attempt harness with params.sourceReplyDeliveryMode = "message_tool_only".
  • It queues a source reply with the same delivery mode through queueAgentHarnessMessage.
  • It asserts the queue accepts the reply and the app-server client receives one turn/steer request containing the completion text.

Security Impact

Low. This preserves an existing run-scoped delivery-mode fact on the active handle. It does not weaken the existing guard: source replies that request message_tool_only are still rejected unless the run explicitly carries that same mode.

Human Verification

Human review should focus on whether this is the intended narrow contract alignment for Codex app-server runs, rather than expanding broader source-reply fallback behavior. Related final-message and requester-wake policy issues are intentionally out of scope.

Risks

  • The change does not address broader cases where the active turn is gone or the final message still needs a message-tool fallback.
  • The proof is source-runtime proof, not a live Discord E2E.
  • If another Codex app-server caller constructs active handles elsewhere, it would need the same field, but this is the active handle used by runCodexAppServerAttempt.

Validation

corepack pnpm format:check -- extensions/codex/src/app-server/run-attempt.ts extensions/codex/src/app-server/run-attempt.test.ts

Passed.

PATH="$PWD/.artifacts/bin:$PATH" corepack pnpm test extensions/codex/src/app-server/run-attempt.test.ts -t "message-tool-only steering"

Passed: 1 test passed, 218 skipped.

node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts -t "message-tool-only steering"

Passed: 1 test passed, 218 skipped.

OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree PATH="$PWD/.artifacts/bin:$PATH" corepack pnpm check:changed --base upstream/main

Passed.

codex review --base upstream/main

Passed: no actionable correctness issues found.

Validation note: the full extensions/codex/src/app-server/run-attempt.test.ts file currently has one unrelated isolated failure in passes session plugin app policy context to elicitation handling (calendarPolicy?.pluginName was undefined). The new focused regression test passes independently.

@openclaw-barnacle openclaw-barnacle Bot added extensions: codex size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 24, 2026, 10:27 PM ET / 02:27 UTC.

Summary
The PR propagates sourceReplyDeliveryMode onto the Codex app-server active-run handle and adds focused regression coverage for message-tool-only queued steering.

PR surface: Source +1, Tests +38. Total +39 across 2 files.

Reproducibility: yes. Current-main source shows the Codex app-server handle lacks the delivery-mode field while the shared queue rejects message-tool-only wakeups without it; the PR’s focused test encodes that source-level path.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
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.

Mantis proof suggestion
A real Discord group-chat demonstration would materially complement the source-runtime proof for the linked user-visible delivery failure. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify a Discord group chat Codex app-server run receives a sub-agent completion after sessions_yield without source_reply_delivery_mode_mismatch.

Risk before merge

  • The supplied proof covers the source-runtime queue contract but does not exercise a live Discord group-chat round trip.
  • The PR body notes one unrelated isolated failure in the full Codex app-server test file, so maintainers should rely on the focused regression and changed-gate evidence for this patch until that separate failure is handled.

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrow Codex app-server contract fix after normal maintainer review, then close the linked Discord sub-agent completion issue as fixed by the merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No ClawSweeper repair job is needed because the branch already contains the narrow code change and regression test; remaining work is maintainer review, CI, and optional live transport proof.

Security
Cleared: The diff only changes Codex runtime TypeScript and a focused test; it does not touch dependencies, workflows, secrets, permissions, or package resolution.

Review details

Best possible solution:

Land this narrow Codex app-server contract fix after normal maintainer review, then close the linked Discord sub-agent completion issue as fixed by the merge.

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

Yes. Current-main source shows the Codex app-server handle lacks the delivery-mode field while the shared queue rejects message-tool-only wakeups without it; the PR’s focused test encodes that source-level path.

Is this the best way to solve the issue?

Yes. The patch is the narrow maintainable fix: carry an existing run-scoped fact onto an existing handle field and keep the existing mismatch guard intact.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4798264a2993.

Label changes

Label justifications:

  • P1: The PR fixes a broken active agent/channel workflow where sub-agent completion wakeups can be rejected and not reach the requester.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies terminal after-fix proof showing the mismatched handle is still rejected and the matched Codex handle queues and sends turn/steer.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies terminal after-fix proof showing the mismatched handle is still rejected and the matched Codex handle queues and sends turn/steer.
Evidence reviewed

PR surface:

Source +1, Tests +38. Total +39 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 0 +1
Tests 1 38 0 +38
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 39 0 +39

What I checked:

Likely related people:

  • Peter Steinberger: Git history on the Codex app-server files includes the original app-server controls work and multiple recent lifecycle/app-server fixes by this author. (role: feature owner and recent area contributor; confidence: high; commits: 31a0b7bd42a5, d9af23fb5af5, 659bcc5e5b59; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/run-attempt.test.ts)
  • Vincent Koc: Current checked-out blame for the shared queue guard, Codex active-handle skeleton, and sibling source-reply contract lines points to recent work by this author, with adjacent Codex harness/auth commits in the same file history. (role: recent area contributor; confidence: medium; commits: ad71a998ff94, f1cc8f0cfc7c, b5dfeaab4ce3; files: extensions/codex/src/app-server/run-attempt.ts, src/agents/pi-embedded-runner/runs.ts, src/agents/pi-embedded-runner/run-state.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: 🦞 diamond lobster Very strong PR readiness with only minor 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 May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: ✨ glimmer Sunspot Proofling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: ✨ glimmer.
Trait: watches the merge queue.
Image traits: location release reef; accessory review stamp; palette sunrise gold and clean white; mood bright-eyed; pose sitting proudly on a smooth stone; shell smooth pearl shell; lighting gentle morning glow; background little resolved-comment flags.
Share on X: post this hatch
Copy: My PR egg hatched a ✨ glimmer Sunspot Proofling in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@ferminquant
ferminquant marked this pull request as ready for review May 25, 2026 02:21
@Takhoffman

Takhoffman commented May 25, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented May 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper automerge is enabled.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

  • 2026-05-25 04:15:16 UTC review queued 8baa1a44399b (queued)

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 25, 2026
@ferminquant
ferminquant force-pushed the codex/issue-86232-source-reply-mode branch from 8baa1a4 to 727047f Compare May 25, 2026 04:20
@clawsweeper clawsweeper Bot mentioned this pull request May 25, 2026
Merged
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the work here. GitHub would not let ClawSweeper push to this branch with the available credentials, so the fix moved to a narrow replacement PR. nothing personal, just permission currents.

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: #86325
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 source PR because this run explicitly enabled source-PR closeout.
The original contribution stays credited in the replacement PR context.
Co-author credit kept:

fish notes: model gpt-5.5, reasoning high; reviewed against e7fdf7b.

@clawsweeper clawsweeper Bot closed this May 25, 2026
@ferminquant
ferminquant deleted the codex/issue-86232-source-reply-mode branch May 25, 2026 17:21
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

clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge extensions: codex P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Sub-agent completion auto-announce silently fails in Discord group chats (source_reply_delivery_mode_mismatch)

2 participants

@ferminquant @Takhoffman