Repository navigation
fix(loop-guard): O96 do not treat lint/test non-zero exits as broken exec - #97
Merged
Merged
Conversation
Normal completed exec/run_tests non-zero exits (lint/test fail) no longer increment byToolFailure or trigger STOP calling it. Timeouts, start failures, kills, and unusable empty output still count. Same-command identical-output streaks use a separate counter with a change-code/command hint (zh+en). Does not touch src/safety or raise failure limits.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Production exec ProcessError timeouts return "timed out after Nms" without an Error: prefix, so is_error stayed false and loop-guard treated them as success — falling into the identical-input generic message without 超时. - isStringToolFailureResult: recognize timed out after / Command killed by - recordToolLoopOutcome: classify VERIFY_SHELL faults even when isError=false - e2e: production-shaped + real execTool timeout through executeOneToolCall
…or=false text scan Child stdout containing 'timed out after' / 'killed by SIG' / ENOENT / 'command not found' was misclassified as a tool fault. Only trust is_error from execute-tool-call's anchored isStringToolFailureResult; exit_code: N header => normal_nonzero regardless of stdout. Adds 4 negative tests (QA).
…ark exec timeout/kill structurally
- classifyShellToolFailure: first-line exit_code 126/127 (and Command failed
(exit 126|127)) => start_failure (未能启动). Stdout never classifies.
- Block copy names the missing command (parsed from shell 'not found' output,
display-only, [A-Za-z0-9._/-]{1,64}) with next step 装依赖或改命令; otherwise
generic 未能启动.
- Remove 'timed out after' / 'Command killed by' text regexes from
isStringToolFailureResult; exec's ProcessError timeout/signal branch calls
ctx.markResultError() so is_error comes from process facts, not stdout.
5 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
O96 — loop-guard false positive: normal
exec/run_testsnon-zero exits (lint/test/check failed) must not count as tool failures and must not produce STOP calling exec / tool unavailable.classifyShellToolFailure: classify by result facts only (normal_nonzerovstimeout/start_failure/killed/unusable/other)recordToolLoopOutcome:exec/run_testsnormal_nonzeroskipsbyToolFailure; tracksbySameCommandFailure(normalized command + output fingerprint)DEFAULT_TOOL_FAILURE_LIMIT; does not touchsrc/safety; discovery / edit thrash / web_fetch rules unchangedSpec
/workspace/specs/O96.md(PM + UX + QA sections). Grok draft:/workspace/specs/O96-grok.md.Revert-red (QA #1 / 验收 D)
Simulated reverting “normal_nonzero skips byToolFailure” (still
byToolFailure +1on that path):Verified: A goes red; B stays green.
Test plan
node --test test/o96-loop-guard-exec-exit.spec.mjs→ 12/12 passnpm run check(format / lint / typecheck) → greennpm test/ CIFiles
src/core/tools/tool-loop-guard.tstest/o96-loop-guard-exec-exit.spec.mjsBase:
c71cd084(post-O99 #96). Head:72b89f1d. Nosrc/safetychanges. Do not merge until QA/UX sign.