Skip to content

fix(release): accept tool-only completion signal#102600

Merged
steipete merged 2 commits into
mainfrom
codex/release-test-hardening
Jul 9, 2026
Merged

fix(release): accept tool-only completion signal#102600
steipete merged 2 commits into
mainfrom
codex/release-test-hardening

Conversation

@steipete

@steipete steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where the OpenAI gateway-profile release lane failed after a successful intentionally tool-only turn because agent.wait returned the known malformed { status: "error", error: "completed" } envelope.

Why This Change Was Made

The live harness now accepts that envelope only when assistant text is explicitly optional. Required-text turns and every other error remain strict, and the following turn must still prove that the tool state is usable.

User Impact

No runtime behavior changes. Release validation no longer reports a false failure for successful tool-only OpenAI turns.

Evidence

@steipete steipete self-assigned this Jul 9, 2026
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS maintainer Maintainer-authored PR labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 5:39 AM ET / 09:39 UTC.

Summary
The PR updates src/gateway/gateway-models.profiles.live.test.ts so optional assistant-text live turns accept a malformed agent.wait completion envelope and retry an additional transient OpenAI tool-read failover message.

PR surface: Tests +45. Total +45 across 1 file.

Reproducibility: yes. The linked focused release run shows the OpenAI gateway-profile lane failing on agent.wait error ... error=completed, and current main's wait helper throws on every non-ok wait result.

Review metrics: 1 noteworthy metric.

  • Completion Envelope Acceptance: 1 optional wait success path added. The new path changes release validation semantics for agent.wait errors and needs proof that it cannot hide a broken tool-only turn.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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:

  • Change the follow-up proof so it does not include the expected nonce values, or verify the first turn's tool transcript/effect directly.
  • [P1] Add a completed after-fix focused live run or log showing the OpenAI gateway-profile tool-only case passing with the strengthened proof.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR links a before-fix live failure and a canceled after-fix Testbox run, but I found no completed after-fix live run showing the OpenAI gateway-profile tool-only case passing. 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.

Risk before merge

  • [P1] Merging as-is could make release validation accept a tool-only completion without independently proving the first turn actually used or preserved the tool-read result.
  • [P1] The linked after-fix Testbox proof is canceled, so there is no completed live OpenAI gateway-profile pass for the changed behavior.

Maintainer options:

  1. Strengthen the live proof before merge (recommended)
    Change the tool-only follow-up or transcript inspection so the release lane proves the earlier tool read without embedding the answer, then rerun focused OpenAI gateway-profile proof.
  2. Accept the release-harness blind spot
    Maintainers could intentionally accept that this lane only confirms terminal completion, but it would no longer prove the tool-only read path described in the PR body.
  3. Pause until live proof is available
    Hold the PR until a completed after-fix live run shows the OpenAI tool-only completion case passing with the corrected proof path.

Next step before merge

  • [P1] The PR needs author or maintainer follow-up for the review finding and completed after-fix live proof; missing contributor proof makes it unsuitable for an automatic repair lane.

Security
Cleared: The diff only changes a live test harness and does not add dependency, workflow, secret-handling, or production code-execution surface.

Review findings

  • [P2] Prove the tool-only read before accepting completion — src/gateway/gateway-models.profiles.live.test.ts:2994
Review details

Best possible solution:

Keep the malformed completion tolerance scoped to optional tool-only turns, but require independent tool-effect proof before accepting the run, such as checking transcript/tool events or asking a follow-up that does not embed the expected nonce values.

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

Yes. The linked focused release run shows the OpenAI gateway-profile lane failing on agent.wait error ... error=completed, and current main's wait helper throws on every non-ok wait result.

Is this the best way to solve the issue?

No. The patch is a plausible harness mitigation, but the best fix must make the follow-up proof independent of the expected nonce values or inspect the tool transcript/effect before accepting the malformed completion envelope.

Full review comments:

  • [P2] Prove the tool-only read before accepting completion — src/gateway/gateway-models.profiles.live.test.ts:2994
    The new allowCompletedWithoutReply path accepts { status: "error", error: "completed" } as success, but the only claimed follow-up proof later tells the model the exact nonce values to reply with. That means the live lane can pass even if the first tool-only turn never read the file or preserved tool state. Please verify the first turn's tool transcript/effect, or make the follow-up recover the nonces without embedding the answer before accepting this completion envelope.
    Confidence: 0.87

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 6433b8272350.

Label changes

Label changes:

  • add P2: This is a normal-priority release-harness bug fix with limited blast radius, but it can affect maintainer confidence in live gateway validation.
  • add merge-risk: 🚨 automation: The diff changes release validation behavior and could let an automated live lane pass without proving the tool-read behavior it is meant to cover.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add 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 links a before-fix live failure and a canceled after-fix Testbox run, but I found no completed after-fix live run showing the OpenAI gateway-profile tool-only case passing. 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.

Label justifications:

  • P2: This is a normal-priority release-harness bug fix with limited blast radius, but it can affect maintainer confidence in live gateway validation.
  • merge-risk: 🚨 automation: The diff changes release validation behavior and could let an automated live lane pass without proving the tool-read behavior it is meant to cover.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • 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 links a before-fix live failure and a canceled after-fix Testbox run, but I found no completed after-fix live run showing the OpenAI gateway-profile tool-only case passing. 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:

Tests +45. Total +45 across 1 file.

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

What I checked:

  • Repository policy applied: Root policy required full AGENTS.md, scoped gateway guidance, Codex contract inspection, and proof checks before a PR verdict. (AGENTS.md:10, 6433b8272350)
  • Scoped gateway policy applied: The gateway guide treats live gateway tests as a special hot path and calls for affected gateway test verification. (src/gateway/AGENTS.md:17, 6433b8272350)
  • Patch accepts malformed completion: The PR adds allowCompletedWithoutReply and treats { status: "error", error: "completed" } as success only through that new optional path. (src/gateway/gateway-models.profiles.live.test.ts:2994, 68b9390da476)
  • Follow-up proof embeds the answer: The tool-only follow-up prompt includes the expected nonce values in the prompt, so it can pass even if the earlier tool-only turn did not preserve or prove tool state. (src/gateway/gateway-models.profiles.live.test.ts:4123, 68b9390da476)
  • Current main reproduces the strict wait failure from source: Current main's live wait helper returns only on status === "ok" and throws formatted failures for all other agent.wait payloads. (src/gateway/gateway-models.profiles.live.test.ts:2898, 6433b8272350)
  • Before-fix live failure observed: The linked focused release run failed the OpenAI gateway profile lane with agent.wait error ... (error=completed) after 109 of 110 tests passed. (42a8679c4dfe)

Likely related people:

  • vincentkoc: Recent path history includes multiple live gateway profile harness commits, including the tool-only live turn and terminal transcript projection work this PR builds on. (role: recent area contributor; confidence: high; commits: 6801d34d9c3b, 3442a8e6f5da, 1f49d94bb296; files: src/gateway/gateway-models.profiles.live.test.ts)
  • jalehman: Authored and merged the canonical transcript reader identity refactor that is adjacent to the live harness transcript verification path. (role: adjacent owner; confidence: medium; commits: d216f7c876dd; files: src/gateway/gateway-models.profiles.live.test.ts)
  • shakkernerd: Recent gateway profile environment restoration work touches the same release-profile test surface and may help route live-profile harness follow-up. (role: recent adjacent contributor; confidence: medium; commits: aa56abc94a2e, eac1d3349c29; files: src/gateway/gateway-models.profiles.live.test.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 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready at exact head 68b9390da4769d817cd33cb14dfb6e2bc02a0db8.

What changed:

  • Intentionally tool-only live turns accept only the known malformed status=error,error=completed terminal envelope; all other errors remain fatal, and the following turn still proves tool state.
  • Tool-read probes retry the provider's explicit transient internal-error response under the existing tool-read + FailoverError + three-attempt guard.
  • Added positive and negative classifier coverage for both boundaries.

Proof:

Known proof gaps: none for the changed surface.

@steipete
steipete merged commit 60bef7b into main Jul 9, 2026
196 of 200 checks passed
@steipete
steipete deleted the codex/release-test-hardening branch July 9, 2026 09:46
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

vincentkoc pushed a commit that referenced this pull request Jul 9, 2026
* test(release): accept tool-only completion signal

* test(release): retry transient tool-read failures

(cherry picked from commit 60bef7b)
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
* test(release): accept tool-only completion signal

* test(release): retry transient tool-read failures
RomneyDa pushed a commit that referenced this pull request Jul 15, 2026
* test(release): accept tool-only completion signal

* test(release): retry transient tool-read failures

(cherry picked from commit 60bef7b)
RomneyDa added a commit that referenced this pull request Jul 15, 2026
* test(release): allow tool-only live turn without transcript

(cherry picked from commit 6801d34)

* test(live): validate final tool probe reply

(cherry picked from commit 3442a8e)

* test(live): wait for terminal transcript projection

(cherry picked from commit 1f49d94)

* fix(release): accept tool-only completion signal (#102600)

* test(release): accept tool-only completion signal

* test(release): retry transient tool-read failures

(cherry picked from commit 60bef7b)

* test(live): correlate tool-read retries (#103204)

Partial backport of the v6.11-compatible transcript correlation and completed-without-reply retry contract only.

(cherry picked from commit 8511585)

* docs(changelog): refresh 2026.6.33 record

---------

Co-authored-by: Vincent Koc <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. P2 Normal backlog priority with limited blast radius. 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