fix(goals): decode quality-gate output as UTF-8 instead of the process codepage - #80236
Closed
Drexuxux wants to merge 1 commit into
Closed
fix(goals): decode quality-gate output as UTF-8 instead of the process codepage#80236Drexuxux wants to merge 1 commit into
Drexuxux wants to merge 1 commit into
Conversation
…s codepage A gate runs whatever command the operator configured, so its output is arbitrary bytes. run_gate captured it with text=True and no encoding, which decodes with locale.getpreferredencoding() under errors="strict". One byte the decoder rejects — a test runner's checkmarks or CJK on a non-UTF-8 Windows console, a stray binary byte anywhere in the stream — kills subprocess's reader thread. proc.stdout comes back None, the `or ""` fallback turns that into an empty tail, and an unhandled traceback is dumped to stderr. The gate's pass/fail verdict still lands on the exit code, but the output tail is exactly what the retry prompt feeds back so the agent can fix the failure. With it empty the agent is told a gate failed and given nothing to act on, so it burns every retry and the goal auto-pauses. workspace_fingerprint has the same two calls; there a non-ASCII path in `git status --porcelain` empties the fingerprint, silently disabling the unchanged-gate skip that exists to stop a stalled agent re-running the same red suite. Decode as UTF-8 with errors="replace" — what git and modern toolchains emit, and what 262 of the repo's 299 text-mode subprocess calls already do.
Contributor
|
Merged in #81963 — clean cherry-pick with authorship (goals run_gate + workspace_fingerprint decodes). Thanks! |
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
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.
What
A
/goalquality gate runs whatever command the operator configured, so its output is arbitrary bytes.run_gatecaptured it in text mode with no encoding:That decodes with
locale.getpreferredencoding()under the defaulterrors="strict". One byte the decoder rejects — a test runner's checkmarks or CJK on a non-UTF-8 Windows console, a stray binary byte anywhere in the stream — kills subprocess's reader thread.proc.stdoutcomes back asNone, theor ""fallback right below turns that into an empty tail, and an unhandled traceback lands on stderr:The verdict itself survives — it rides on the exit code. What is lost is the tail, and the tail is the whole point of the retry loop:
evaluate_gatesstores it aslast_output_tailand feeds it back so the agent can fix what broke. Empty, the agent is told a gate failed and handed nothing to act on, so it burnsmax_retriesand the goal auto-pauses withquality gate exhausted N retries.workspace_fingerprinthas the same two calls. There a non-ASCII path ingit status --porcelainempties the fingerprint, which silently disables the unchanged-gate skip — the rule that exists so a stalled agent can't spin re-running an identical red suite.The fix
Decode as UTF-8 with
errors="replace"at all three call sites. That is what git and modern toolchains actually emit, and it is already the house style: 262 of the repo's 299 text-mode subprocess calls pass an explicit encoding.errors="replace"also means genuinely undecodable bytes degrade to replacement characters instead of destroying the surrounding diagnostics.Same class as the open
working_diffand cron-script decode fixes; this is the/goalgate path, which none of them touch.Tests and results
test_run_gate_keeps_diagnostics_when_a_byte_will_not_decoderuns a gate that prints a realistic failure line with one undecodable byte embedded and exits 1, then asserts the diagnostic text reaches the tail. It fails onmain—AssertionError: gate diagnostics were lost to a decode failure (tail='')— and passes with the fix. The byte chosen is invalid under strict UTF-8 as well as the Windows ANSI codepages, so the test pins the behaviour on Linux CI, not only on the platform where I found it.The three goals suites (
test_goal_gates.py,test_goals.py,test_kanban_goal_mode.py) run to 57 passed with the fix versus 56 onmain— the one added test — with the same single pre-existing failure both ways:test_run_gate_fail_captures_output, which shells out with POSIX syntax (echo broken >&2; exit 3) and does not run on a Windows shell. It is untouched by this change.