Skip to content

fix(xai): preserve plaintext string child-result continuation - #3991

Merged
lidge-jun merged 7 commits into
devfrom
codex/bug6-01a07e9d-2-xai
Sep 8, 2026
Merged

fix(xai): preserve plaintext string child-result continuation#3991
lidge-jun merged 7 commits into
devfrom
codex/bug6-01a07e9d-2-xai

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

A routed Grok parent can receive a completed child's result as a string-valued Codex agent_message. The existing non-forward conversion handled arrays only, leaving that string envelope on xAI's strict Responses wire and causing 422 errors. Convert nonblank strings through the same user-message representation only for the existing exact xAI destination predicate; preserve exact text, forward traffic, arrays, and mixed-ciphertext rejection.

Closes #3907.

Manual stack, layer 2 of 6, depends on #3986: Go compatibility → xAI child result → V2 guidance → server preset → canonical reset alias → cooldown recovery. Review only this layer; merge bottom-up after retargeting to dev.

Verification

  • Local product tests, suites, install, typecheck and build: NOT RUN, per owner instruction. Git hooks disabled per invocation; push uses --no-verify.
  • Production-adapter destination controls and synthetic HTTP SSE/JSON parent → child → parent-result scenarios retain genuine tool-call/result pairs and reject surviving private items. These simulate client envelopes; they do not execute the native Codex scheduler or real OAuth/inference.
  • Independent Astra high plan/source/security audit passed on prepared candidate 339e42c1e; actual stack head 00eb47886690e7b24b0eed69b6d870c33ceade62 has identical product/docs trees (checked with git diff). Hosted CI34180674115 is running.
  • Remote documentation build on Bun1.4.0/Node24.20.0 passed425pages for docs tree d57460224795b9224c5bcda555be2eadcd8974a2, using an isolated macmini-cf scratch directory and verified archive hash. No deployment.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Maintainer integration decision

The owner explicitly authorized bottom-up integration of this manual stack into dev. Acting as current maintainer lidge-jun, I choose the dev-only maintainer-integration path in MAINTAINERS.md; this is not self-approval. Independent technical/security review and contributor attribution remain required, and any maintainer objection must be resolved.

This PR's certified candidate head is 00eb47886690e7b24b0eed69b6d870c33ceade62 with PR CI34180674115. Cumulative integration head f80f39d20e8395901d3b62758d118ea3a559a9f4 contains current dev 402be7c1f88283eb8465c3aec8437ccecd2542ec; full lane=all run34193218874 is the required final matrix. PR CI34193213502 attempt2 passed after one investigated macOS job cancellation; the full dispatch reran only its failed macOS control after same-head shard evidence passed. Failed attempts remain recorded. These links identify the exact evidence to inspect; any pending, failed, cancelled or skipped required execution blocks landing. The actor, base, head, reviews and checks will be refreshed immediately before each merge.

Serial merge prediction is conflict-free and its final tree equals the cumulative candidate. Because merged branches are automatically deleted, the next owned child is retargeted to dev immediately before its parent lands. PRs remain ordinary/manual; no native stack registration is requested. Local product checks remain NOT RUN by owner instruction.

Final pre-landing verification: full run34193218874 attempt2 SUCCESS, all26 named jobs and mandatory execution steps verified at f80f39d20e8395901d3b62758d118ea3a559a9f4; PR run34193213502 attempt2 SUCCESS. The investigation allowed one retry of each failed/cancelled macOS job; prior failures remain historical, and previously passing jobs were not rerun. Current CI is accepted for this owner-authorized integration.

t and others added 7 commits September 8, 2026 10:42
Reimplements remaining placement and stateless intent from #3838 at d84e5a8. Preserves the already-landed fail-closed agent-message contract.

Co-authored-by: jpierrevd <265811239+jpierrevd@users.noreply.github.com>
Refs #3907. Enable exact string preservation through the existing agent-message normalizer only for non-forward xAI destinations. Keep array conversion and mixed-ciphertext fail-closed behavior unchanged.

Add adapter destination controls and mocked parent/child/result continuation coverage for SSE and JSON. This is an isolated WP2 preparation candidate; local product tests, typecheck, build, install and runtime probes were NOT RUN. Main owns phase adoption, independent audit and hosted CI.

(cherry picked from commit 339e42c1e388db1ca01cda9d960368ad9335d48c)
@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c97c36aa-52bb-47f5-b246-9067911c9c5d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 8, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 72 / 80

이 PR은 라우팅된 Grok(xAI Responses) 부모 스레드가 자식 결과를 Codex 전용 agent_message로 다시 보낼 때, 내용이 문자열이거나 배열이거나에 따라 깨지던 구멍을 막습니다. 지금 dev HEAD(4d4629024, 2.48.0)의 src/adapters/routed-agent-messages.tscontent비어 있지 않은 배열이고 각 조각이 input_text/input_image/input_file일 때만 공개 user message로 바꿉니다. 문자열이 오면 그대로 두고, xAI 쪽은 그 private 타입을 422 unknown item type "agent_message"로 거절합니다. 이슈 #3907이 바로 그 증상입니다. Codex는 매 턴 서브에이전트 이력을 다시 실어 보내므로, 한 번 붙은 문자열 결과는 그 스레드가 끝날 때까지 반복 실패합니다.

이번 변경은 세 층으로 나뉩니다. (1) normalizeRoutedAgentMessages에 기본값 falseallowStringContent 옵션을 추가하고, 켠 경우에만 공백만이 아닌 문자열을 정확히 같은 글자(앞뒤 공백·개행 포함)로 input_text 한 조각으로 감쌉니다. (2) createResponsesPassthroughAdapter의 non-forward 경로에서만 isXaiResponsesDestination(provider)가 참일 때 그 옵션을 켭니다. 이 판별은 이미 HEAD에 있는 src/providers/xai-transport.ts의 HTTPS api.x.ai / cli-chat-proxy.grok.com + 표준 443 포트 exact-host 규칙입니다. (3) 어댑터·서버 픽스처로 부모→자식→부모결과 SSE/JSON과 목적지 negative(forward, OpenCode Go, evil lookalike host, http, nonstandard port)를 덮습니다. docs와 structure/04_transports-and-sidecars.md도 같은 범위를 말합니다.

현재 dev와의 관계는 중요합니다. 이 PR의 base는 dev가 아니라 codex/bug6-01a07e9d-1-go이고, 본문이 말하는 수동 스택(Go 호환 → xAI 자식 결과 → V2 guidance → …)의 2층입니다. #3986(Go 층)에 의존한다고 적혀 있고 draft입니다. 제품 로컬 typecheck/build/install은 작성자 지시로 NOT RUN입니다. Cross-platform CI의 Linux test shard는 녹색이었고 macOS는 리뷰 시점 기준 진행 중이었습니다. 암호화/혼합 조각은 기존 fail-closed를 유지하고, forward auth는 옵션을 켜지 않습니다. 즉 “xAI에서 문자열 child result만 공개 메시지로 낮춘다”는 범위는 HEAD 카탈로그·transport 소유권과 잘 맞습니다. 다만 단독으로 dev에 바로 머지할 형태는 아직 아닙니다.

라인 src/adapters/openai-responses.ts (normalize 호출부) - non-forward일 때만 allowStringContent: isXaiResponsesDestination(provider)를 넘기므로 범위는 맞지만, base가 스택 중간 브랜치라 dev에 리타깃하기 전에 #3986 층이 먼저 안착했는지 확인해야 한다
라인 src/adapters/routed-agent-messages.ts (문자열 분기) - trim은 “비어 있나”만 보고 원문 바이트는 유지한다. 공백만 문자열은 변환하지 않아 422가 남을 수 있는데, 의도적 fail-closed로 보이며 문서에도 적혀 있다
경로 tests/adapters/routed-agent-messages.test.ts / tests/server/server-xai-responses-streaming.test.ts - 목적지 negative와 SSE/JSON 연속성은 좋지만, 작성자가 로컬 전체 typecheck/build를 돌리지 않았다고 명시했다. CI 전체(특히 macOS) 마감을 머지 게이트로 둬야 한다
경로 PR base codex/bug6-01a07e9d-1-go - dev가 아니므로 지금 머지하면 스택 순서·리타깃 규칙을 깨기 쉽다
경로 이슈 #3907 - Closes로 연결됨. 스택이 dev에 안착하기 전에는 이슈만 닫지 말고 PR 머지와 함께 닫는 편이 맞다

메인테이너의 판단이 필요한 지점

  • 수동 스택을 bottom-up으로 유지할지: #3986을 먼저 dev에 올린 뒤 이 PR을 dev로 리타깃할지, 아니면 스택 브랜치 통째 랜딩 PR로 묶을지
  • draft + 로컬 NOT RUN을 허용한 채 CI만으로 ready로 올릴지, 아니면 작성자(또는 메인테이너)가 로컬 게이트를 한 번 더 돌릴지
  • 문자열 변환을 xAI exact-host에만 둘지, 같은 422를 내는 다른 Responses 목적지까지 넓힐지(지금은 의도적으로 좁음)

너의 추천
스택 순서대로 #3986(또는 동등한 Go 층)이 dev에 안착한 뒤 이 PR을 dev로 리타깃하고, draft 체크리스트·CI(macOS 포함) 마감 후 ready로 올린 다음 머지하세요. #3907은 그때 Closes로 함께 닫으면 됩니다. 지금 중간 base에 단독 머지하지 마세요.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun
lidge-jun marked this pull request as ready for review September 8, 2026 05:23
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 8, 2026 05:23
@lidge-jun
lidge-jun changed the base branch from codex/bug6-01a07e9d-1-go to dev September 8, 2026 07:02
@lidge-jun
lidge-jun merged commit 7730f66 into dev Sep 8, 2026
46 checks passed
@lidge-jun
lidge-jun deleted the codex/bug6-01a07e9d-2-xai branch September 8, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant