Skip to content

v0.8.3 — JSONL record integrity, UTF-8 report, encode-before-spawn, \Z role ids - #7

Merged
ehzawad merged 1 commit into
mainfrom
agent/v0.8.3-jsonl-locale-robustness
Jul 12, 2026
Merged

v0.8.3 — JSONL record integrity, UTF-8 report, encode-before-spawn, \Z role ids#7
ehzawad merged 1 commit into
mainfrom
agent/v0.8.3-jsonl-locale-robustness

Conversation

@ehzawad

@ehzawad ehzawad commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Four robustness fixes from the council review. Each was reproduced before fixing; regression tests added for all four (268 passing, ruff clean).

Fixes (codex_council.py)

  1. JSONL records with U+2028/U+2029/U+0085 no longer dropped_iter_json_objects used str.splitlines(), which breaks on those three chars even though they're legal unescaped inside a JSON string. codex can emit an agent_message containing one literally, so the record was torn into invalid fragments and the completed reply silently discarded → role reported FAILED. Now splits strictly on "\n".
  2. Report + sentinel survive a strict C locale_force_utf8_streams() pins stdout/stderr to UTF-8 at startup. Under LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0, stdout defaulted to ASCII and the report header's em dash raised UnicodeEncodeError, losing both the report and the CODEX_COUNCIL_DONE sentinel (even with ASCII role replies).
  3. No leaked codex child on an un-encodable prompt_run_codex_subprocess now encodes the prompt before create_subprocess_exec and reaps the child on any post-spawn BaseException, not just CancelledError. A lone surrogate (e.g. escaped \uD800 in roles.json) previously raised after the child existed, leaving codex blocked forever on stdin.
  4. Role ids can't smuggle a trailing newlineROLE_ID_PATTERN uses \Z instead of $ (which matches before a final \n), so "architect\n" no longer reaches state filenames / report / progress lines.

Tests

  • Unit: literal-separator preservation (U+2028/2029/0085), encode-before-spawn (asserts no spawn), reap-on-non-cancel-error, _force_utf8_streams behavior, trailing/embedded-newline role ids.
  • CLI: full council run under LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 asserting exit 0, report + em dash present, and the genuine sentinel.

Follows v0.8.2 (#6).

🤖 Generated with Claude Code

…fore-spawn; \Z role ids

Four robustness fixes surfaced by the council review (v0.8.3). All four were
reproduced before fixing.

- _iter_json_objects split on "\n" instead of str.splitlines(): splitlines
  also breaks on U+2028/U+2029/U+0085, which are legal *unescaped* inside a
  JSON string. codex can emit an agent_message containing one literally, and
  splitting there tore the record into invalid fragments — silently dropping a
  completed reply and reporting the role as failed.

- _force_utf8_streams() pins stdout/stderr to UTF-8 at startup. Under a strict
  C locale with UTF-8 mode and C-coercion both disabled, the report header's em
  dash raised UnicodeEncodeError and lost BOTH the report and the
  CODEX_COUNCIL_DONE sentinel; ASCII input alone still triggered it.

- _run_codex_subprocess encodes the prompt BEFORE create_subprocess_exec and
  reaps the child on any post-spawn BaseException, not just CancelledError. A
  lone surrogate (e.g. escaped "\uD800" in roles.json) previously raised after
  the child existed, leaking a codex process blocked forever on stdin.

- ROLE_ID_PATTERN uses \Z instead of $, so "architect\n" (which $ accepts,
  matching before a trailing newline) can no longer inject a newline into
  state filenames and report/progress lines.

Adds regression tests for all four (unit + a strict-C-locale CLI test); 268
passing, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehzawad
ehzawad merged commit 8c3ff9e into main Jul 12, 2026
4 checks passed
@ehzawad
ehzawad deleted the agent/v0.8.3-jsonl-locale-robustness branch July 12, 2026 12:04

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several robustness improvements to the Codex Council plugin, including updating the role ID regex to use \Z to prevent trailing newlines, splitting JSONL streams strictly on \n to preserve valid Unicode line separators, encoding prompts before spawning subprocesses to avoid process leaks, catching BaseException during subprocess communication for reliable cleanup, and forcing stdout/stderr streams to UTF-8 to prevent encoding errors in strict C locales. The review feedback suggests broadening the exception handling in _force_utf8_streams to catch Exception instead of just (ValueError, OSError) to ensure compatibility with custom or mock stream implementations, along with a corresponding test update.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1650 to +1651
except (ValueError, OSError):
pass

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since _force_utf8_streams is a best-effort initialization step, it should be extremely robust against any custom or mock stream implementations. Standard streams or third-party wrappers might raise other exceptions like TypeError (if the signature of reconfigure is different or doesn't accept these keyword arguments) or AttributeError. Catching Exception instead of just (ValueError, OSError) ensures that the CLI tool never crashes at startup due to stream reconfiguration failures.

Suggested change
except (ValueError, OSError):
pass
except Exception:
pass

Comment on lines +2710 to +2712
class Boom:
def reconfigure(self, **kw):
raise ValueError("nope")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To verify that _force_utf8_streams is robust against other exceptions (such as TypeError which can occur if a custom stream's reconfigure method has an incompatible signature), we should update this test to raise a TypeError instead of ValueError.

Suggested change
class Boom:
def reconfigure(self, **kw):
raise ValueError("nope")
class Boom:
def reconfigure(self, **kw):
raise TypeError("nope")

ehzawad added a commit that referenced this pull request Jul 12, 2026
v0.8.3 — JSONL record integrity, UTF-8 report, encode-before-spawn, \Z role ids
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant