fix(qa): accept structured gateway restart outcomes#103681
Conversation
openclaw-barnacle
Bot
added
extensions: qa-lab
size: XS
maintainer
![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: 29f0cb6b98
ℹ️ 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".
| - expr: liveTurnTimeoutMs(env, 180000) | ||
| - assert: | ||
| expr: "waited.status === 'ok' || waited.status === 'timeout' || (waited.status === 'error' && (String(waited.error ?? '').includes('EmbeddedAttemptSessionTakeoverError') || String(waited.error ?? '').includes('AbortError') || String(waited.error ?? '').includes('This operation was aborted')))" | ||
| expr: "waited.status === 'ok' || waited.status === 'timeout' || (waited.status === 'error' && (waited.stopReason === 'restart' || waited.stopReason === 'aborted' || String(waited.error ?? '').includes('EmbeddedAttemptSessionTakeoverError') || String(waited.error ?? '').includes('AbortError') || String(waited.error ?? '').includes('This operation was aborted')))" |
There was a problem hiding this comment.
In the runtime-parity suite this same YAML runs once with forcedRuntime=openclaw and once with forcedRuntime=codex (extensions/qa-lab/src/suite.ts:847-888). OpenClaw distinguishes restart aborts from ordinary direct aborts: resolveAgentRunAbortLifecycleFields returns stopReason: "restart" only for restart abort reasons and "aborted" for other aborts (src/agents/run-termination.ts:60-78). With this unconditional branch, an OpenClaw regression that loses restart attribution or directly aborts the in-flight turn still passes this restart-recovery scenario as long as the follow-up turn works, so the release gate can miss the exact gateway-restart contract it is meant to protect. Gate aborted to the Codex forced-runtime cell (or otherwise verify restart ownership) instead.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 8:55 AM ET / 12:55 UTC. Summary PR surface: Source +1, Tests +14, Other 0. Total +15 across 4 files. Reproducibility: yes. Current main deterministically rejects the reported structured results, and the issue supplies exact prior live outputs from both runtime cells; the after-fix path has not yet been demonstrated. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use the forced-runtime fact in the scenario so native OpenClaw requires structured Do we have a high-confidence way to reproduce the issue? Yes. Current main deterministically rejects the reported structured results, and the issue supplies exact prior live outputs from both runtime cells; the after-fix path has not yet been demonstrated. Is this the best way to solve the issue? No. Reading structured terminal metadata is the correct direction, but the current shared assertion accepts a generic native OpenClaw abort and should instead enforce each runtime's canonical restart outcome. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ea6aa1dc7591. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1, Tests +14, Other 0. Total +15 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
Land-ready proof for exact head
No proof gaps remain for this bounded QA release-gate repair. |
|
Merged via squash.
|
1 participant
Closes #103676
What Problem This Solves
Fixes an issue where the live runtime-parity release gate reported gateway restart recovery as failed when
agent.waitreturned the current structuredrestartorabortedterminal reason.Why This Change Was Made
The scenario now accepts only those two canonical restart-owned stop reasons in addition to its existing
ok,timeout, and legacy upgrade-path error forms. Recovery health, interrupted-marker deduplication, and exactly-once recovery delivery remain mandatory. The QA wait-result boundary now types and testsstopReasonpreservation.User Impact
Release validation no longer false-fails a healthy gateway restart solely because current runtimes use structured terminal metadata. Runtime product behavior is unchanged.
Evidence
7b03a2ae201ab9c27dc3bc39569e97c9f9582360: OpenClaw returnedstatus=error, stopReason=restart; Codex returnedstatus=error, stopReason=aborted.pnpm test extensions/qa-lab/src/scenario-catalog.test.ts extensions/qa-lab/src/suite-runtime-agent-process.test.ts: 65/65 passed on Blacksmith Testboxtbx_01kx5y7vzbyq4s1pb1nc2kdakjthrough the repository Crabbox wrapper.pnpm check:changed: typecheck, lint, guards, and runtime import-cycle checks passed on the same Testbox.oxfmtandgit diff --checkpassed.overall: patch is correct, confidence 0.97).29f0cb6b9872a88149aac9e882477f2609225392using Blacksmith Testboxtbx_01kx5xy78gj59snb0ny0tqh2cx.AI-assisted change; implementation and contracts were source-audited, including the direct Codex app-server interrupt contract.