fix(handoffs): keep a nested history record on one line for every line boundary - #4787
Draft
Shinku1337 wants to merge 1 commit into
Draft
fix(handoffs): keep a nested history record on one line for every line boundary#4787Shinku1337 wants to merge 1 commit into
Shinku1337 wants to merge 1 commit into
Conversation
…e boundary The nested handoff history writer chose the compact "role: text" record form whenever _contains_newline was false, but that check only tested "\n" and "\r" while the reader splits records with str.splitlines(), which also splits on vertical tab, form feed, the file/group/record separators, NEL, U+2028, and U+2029. Content holding one of those was written as one record and read back as several. json.dumps(ensure_ascii=False) emits U+0085/U+2028/U+2029 verbatim, so the lossless JSON record form split as well. Derive the writer's gate from the reader's own splitlines() behavior so the two definitions cannot drift, and escape the three boundaries json.dumps leaves raw. Records without such characters serialize byte-identically to before.
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.
Summary
nest_handoff_historyserializes each previous conversation turn as one numbered record inside the generated<CONVERSATION HISTORY>assistant message, and a later handoff parses those records back into transcript items. The writer and the reader disagree about what terminates a record:_format_transcript_itempicks the compactrole: textrecord form whenever_contains_newlineis false, and that check only tests"\n"and"\r"(history.py:411)._split_summary_recordssplits records withstr.splitlines()(history.py:494), which also splits on\v,\f,\x1c,\x1d,\x1e,\x85(NEL), U+2028, and U+2029.Content holding one of those eight characters is therefore written as one record and read back as several. The JSON record form is affected too:
json.dumps(..., ensure_ascii=False)emits U+0085, U+2028, and U+2029 verbatim, so the path chosen specifically to be lossless also splits.Two observable consequences:
rolebecomes the literal string'{"role"'and itscontentbecomes a fragment of the raw JSON — contradicting the propertytest_nest_handoff_history_flattens_structured_content_without_stringifyingalready asserts for"\n"._parse_summary_line(history.py:529) derives a role from the text preceding the first:in each fragment. A turn can therefore appear in the next agent's history carrying a role the sender never sent. Worth noting for applications that forward untrusted end-user text through a nested handoff chain.Scope:
RunConfig.nest_handoff_historyis an opt-in beta and defaults toFalse(run_config.py:374), and consequence 2 additionally requires at least two handoffs in the chain — one to write the summary, one to flatten it.Fix
Two changes in
src/agents/handoffs/history.py, no public API change:_contains_newlinewith_spans_line_boundary(value), defined asbool(value) and value.splitlines() != [value]. Deriving the writer's gate from the same function the reader uses keeps the two definitions from drifting again, and comparing against[value]also catches a trailing boundary, whichsplitlines()drops rather than splits on._escape_json_line_boundaries(), which escapes the three boundariesjson.dumpsleaves raw to�//, and apply it to everyjson.dumpsin this module.json.loadsrestores the original character, so a serialized record stays on one line and the round trip stays lossless.Records that contain none of these characters serialize byte-identically to before, so existing stored summaries stay readable. Text that does contain one is now routed to the existing lossless JSON path instead of being split.
Test plan
Two tests added to
tests/test_extension_filters.py, each parameterized over all eleven boundary forms (\n,\r,\r\n,\v,\f,\x1c,\x1d,\x1e,\x85, U+2028, U+2029):test_nest_handoff_history_keeps_line_boundary_content_in_one_recordtest_nest_handoff_history_keeps_line_boundary_structured_content_losslessBoth fail on the parent commit and pass with this change. Verified deterministic — 5 runs on each side, same result every time. Observed failure on the parent commit:
Commands run and their results:
pytest tests/test_extension_filters.py -q— 45 passed.pytestover the handoff, items,apply_diff,function_schema, andusagesuites — 305 passed.pytest tests -q -m "not serial" --continue-on-collection-errors— the set of failing tests is identical to the parent commit's; a diff of the twoFAILEDlists is empty.ruff format --checkandruff checkon both changed files — clean (ruff 0.9.2, the pinned version).python .github/scripts/check_optional_truthiness.py src/agents— clean.mypy src— 46 errors before, the same 46 after, diff empty;mypy src/agents/handoffs/history.pyalone — clean both before and after.Limitations
Verified on Windows 11, Python 3.11.2, pydantic 2.12.3, openai 3.0.0, pytest 8.4.1, dependencies installed with
uv sync --group dev.I could not complete
.agents/skills/code-change-verification/scripts/run.shormake checkend to end in this environment: several optional test extras are not installable here (httpx,numpy,litellm,docker). Those missing extras account for the pre-existing collection errors and failures noted above — all identical on the parent commit and unrelated to this change. The individualformat,lint,typecheck, andtestssteps were run and their results are listed above. Happy to have anything re-confirmed in CI.Disclosure note
Consequence 2 is security-adjacent, so this description covers the record-boundary defect and its mechanism without a ready-to-run payload. Per
SECURITY.md, the same finding is also being sent to OpenAI's coordinated disclosure address. Glad to close, re-scope, or hold this PR if maintainers would rather handle it privately first.Issue number
N/A — independently reproduced, no matching issue or PR found. I enumerated all 45 open pull requests and the 60 most recently closed ones; none touch
src/agents/handoffs/history.py. Issue and PR searches forU+2028,splitlines,_contains_newline,line separator,vertical tab, andNELreturned nothing describing this defect.Checks
.agents/skills/code-change-verification/scripts/run.sh— blocked by the missing optional extras described under Limitations; the individual format, lint, typecheck, and test steps were run instead/reviewbefore submitting this PR