fix(ralph-loop): detect VERIFIED promise in tool_result parts and add verification circuit breaker (fixes #3212)#3447
Conversation
… verification circuit breaker (fixes code-yeongyu#3212)
|
I have read the CLA Document and I hereby sign the CLA |
![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: 193c8cf5a5
ℹ️ 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".
| } | ||
|
|
||
| state.verification_pending = true | ||
| state.verification_attempts = 0 |
There was a problem hiding this comment.
Resetting verification_attempts to 0 when entering verification mode causes the new circuit breaker in restartAfterFailedVerification to never reach MAX_VERIFICATION_ATTEMPTS in normal ULTRAWORK flow. Each failed verification clears verification_pending, and the next completion path calls markVerificationPending again, which wipes the accumulated count before the next failure is recorded. In repeated Oracle-failure scenarios, this leaves the loop effectively unbounded (until general iteration limits), so the intended protection does not activate.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files
Confidence score: 3/5
verification_attemptsis reset insrc/hooks/ralph-loop/loop-state-controller.ts, which can nullify the new circuit breaker and allow repeated retries beyond the intended limit.- There is a user-facing risk:
src/hooks/ralph-loop/verification-failure-handler.tscan show a misleading “max attempts” toast even when other restart failures occur. - Given a high-severity behavioral issue (7/10) that can change retry control, this carries some regression risk and merits extra verification.
- Pay close attention to
src/hooks/ralph-loop/loop-state-controller.tsandsrc/hooks/ralph-loop/verification-failure-handler.ts- circuit breaker state reset and misleading failure messaging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/ralph-loop/loop-state-controller.ts">
<violation number="1" location="src/hooks/ralph-loop/loop-state-controller.ts:129">
P1: Resetting `verification_attempts` here nullifies the new circuit breaker across verification retries.</violation>
</file>
<file name="src/hooks/ralph-loop/verification-failure-handler.ts">
<violation number="1" location="src/hooks/ralph-loop/verification-failure-handler.ts:86">
P2: The failure toast claims the loop ended because maximum attempts were reached, but `restartAfterFailedVerification()` returns `null` for several other restart failures too, so the user-facing diagnosis can be wrong.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| await ctx.client.tui?.showToast?.({ | ||
| body: { | ||
| title: "ULTRAWORK LOOP", | ||
| message: "Verification failed after maximum attempts. Loop ended.", |
There was a problem hiding this comment.
P2: The failure toast claims the loop ended because maximum attempts were reached, but restartAfterFailedVerification() returns null for several other restart failures too, so the user-facing diagnosis can be wrong.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/ralph-loop/verification-failure-handler.ts, line 86:
<comment>The failure toast claims the loop ended because maximum attempts were reached, but `restartAfterFailedVerification()` returns `null` for several other restart failures too, so the user-facing diagnosis can be wrong.</comment>
<file context>
@@ -77,9 +77,17 @@ export async function handleFailedVerification(
+ await ctx.client.tui?.showToast?.({
+ body: {
+ title: "ULTRAWORK LOOP",
+ message: "Verification failed after maximum attempts. Loop ended.",
+ variant: "error",
+ duration: 10000,
</file context>
| message: "Verification failed after maximum attempts. Loop ended.", | |
| message: "Verification failed. Loop ended.", |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efd6da188a
ℹ️ 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".
| const attempts = (state.verification_attempts ?? 0) + 1 | ||
| if (attempts >= MAX_VERIFICATION_ATTEMPTS) { |
There was a problem hiding this comment.
restartAfterFailedVerification increments state.verification_attempts, but this value is never serialized in writeState/readState (see src/hooks/ralph-loop/storage.ts), so each retry reloads the state with verification_attempts as undefined and this line recomputes attempts as 1 every time. In repeated Oracle verification failures, MAX_VERIFICATION_ATTEMPTS is therefore never reached and the new circuit breaker does not actually stop the retry loop.
Useful? React with 👍 / 👎.
|
Closing — PR has been open 14+ days with zero maintainer engagement. Happy to reopen or rework if this fix is still wanted. |
1 participant
Summary
Problem
The ULW verification loop runs infinitely after Oracle emits
<promise>VERIFIED</promise>because:detectOracleVerificationFromParentSessiononly scansassistantrole messages, missing VERIFIED in tool_result parts delivered via non-assistant messagesdetectCompletionInSessionMessagesfilters to assistant-only messages when searching for the VERIFIED promise, skipping tool_result parts in other message rolesFix
detectOracleVerificationFromParentSessionto skip onlyusermessages instead of requiringassistant, so VERIFIED evidence in tool_result parts is found regardless of message roleULTRAWORK_VERIFICATION_PROMISE,detectCompletionInSessionMessagesnow scans all non-user messages instead of assistant-onlyMAX_VERIFICATION_ATTEMPTS = 3circuit breaker: after 3 failed verification attempts, the loop state is cleared and an error toast notifies the userChanges
src/hooks/ralph-loop/pending-verification-handler.tsuserrole instead of requiringassistantwhen scanning for Oracle VERIFIEDsrc/hooks/ralph-loop/completion-promise-detector.tssrc/hooks/ralph-loop/loop-state-controller.tsrestartAfterFailedVerification, reset counter inmarkVerificationPendingsrc/hooks/ralph-loop/verification-failure-handler.tssrc/hooks/ralph-loop/types.tsverification_attemptsfield toRalphLoopStatesrc/hooks/ralph-loop/constants.tsMAX_VERIFICATION_ATTEMPTSconstantFixes #3212
Summary by cubic
Fix the infinite ULW verification loop by detecting Oracle VERIFIED in non-assistant
tool_resultparts and adding a 3-attempt circuit breaker to stop runaway retries (fixes #3212).usermessages when detectingULTRAWORK_VERIFICATION_PROMISE.tool_resultparts regardless of message role.MAX_VERIFICATION_ATTEMPTS = 3; preserve attempt count across pending states, end loop and show error toast when limit is reached.Written for commit efd6da1. Summary will update on new commits.