Skip to content

fix(responses): compile structured shell payloads sent to code-mode exec - #5352

Closed
Flowershangfromthebranches wants to merge 1 commit into
lidge-jun:devfrom
Flowershangfromthebranches:fix/code-mode-exec-shell-payload
Closed

Flowershangfromthebranches wants to merge 1 commit into
lidge-jun:devfrom
Flowershangfromthebranches:fix/code-mode-exec-shell-payload

Conversation

@Flowershangfromthebranches

@Flowershangfromthebranches Flowershangfromthebranches commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

A routed Chat model can emit exec with a structured shell payload such as {"cmd":"cd \"/tmp/example repo\" && git status --short"}. Codex executes exec as JavaScript: older restoration forwards the JSON, and alternate-field restoration forwards bare shell text. Both fail before the nested shell tool runs.

  • Recognize an unambiguous shell-argument object under a verified code-mode catalog, including the canonical input wrapper, and reuse the existing helper compiler to emit tools.exec_command(...) plus text(result).
  • Preserve shell options and serialize command text as data. Leave valid JavaScript fallback fields (including ambiguous single identifiers), unknown fields, conflicting command fields, flat tool catalogs, and foreign namespaces alone.
  • Hold possible shell-object previews until completion so JSON and SSE paths deliver the same executable input. Ordinary JavaScript remains progressive.
  • Document the behavior in the Responses contract and English/Chinese Codex integration guides.

This fixes the tool-payload compatibility defect observed with StepFun through the shared adapter path. A subsequent provider 429 rate_limit_exceeded is a separate upstream condition: this PR does not change retry policy or claim to remove provider limits.

Verification

  • Reproduced before the fix: the new recognition/compilation regressions failed (2 failures).
  • Focused regression run: 117 passed, 0 failed, across responses-code-mode-shell-compile, responses-code-mode-patch-compile, apply-patch-envelope, responses-custom-tool-repair, responses-custom-tool-stream-consistency, bridge-legacy-shell-normalization, and both test-layout suites.
  • Tests execute generated JavaScript against a stub nested tool, preserving shell metacharacters as arguments; they cover JSON, adapter SSE, native Responses SSE, lowered Responses SSE, and all split boundaries for representative payloads.
  • bun run typecheck — passed.
  • bun test tests/ci-workflows/file-size-ratchet.test.ts — 9 passed.
  • bun run structure:check and bun run privacy:scan — passed.
  • Docs: bun install --frozen-lockfile and bun run build in docs-site — passed (497 pages).
  • git diff --check — passed.
  • Full repository suite and live StepFun/Codex acceptance were not run; validation intentionally stayed focused. Submitted as a draft for the repository's contributor review process.

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.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

이 PR은 모델이 code-mode exec에 자바스크립트 대신 {"cmd":"..."} 같은 셸 인자 객체를 넣었을 때, 그 객체를 다시 tools.exec_command(...) 자바스크립트로 바꿔 주는 수정입니다. 예전에 복원이 JSON을 그대로 넘기거나, 명령 글자만 꺼내 넘기면 Codex가 exec를 자바스크립트로 실행하다 깨졌습니다. 이번 코드는 도구 목록이 진짜 code-mode인지 확인한 뒤에만 동작합니다. cmd/command와 알려진 셸 옵션만 있는 객체만 고치고, 자바스크립트로도 읽히는 값·모르는 키·서로 다른 명령 필드가 같이 있는 경우는 손대지 않습니다. 스트리밍에서는 셸 객체처럼 보이는 미리보기를 끝까지 붙잡아 두고, 끝난 뒤에만 컴파일된 자바스크립트를 보냅니다. 그래서 JSON 경로와 SSE 경로가 같은 실행 입력을 받습니다. 문서(영문·중문 가이드, responses 계약)도 같이 고쳤고, 회귀 테스트가 경계와 분할 지점을 꽤 촘촘히 봅니다. base는 dev입니다. draft이고, StepFun에서 본 호환 깨짐을 고치는 범위로 맞춰 두었습니다. 제공자 429 재시도 정책은 건드리지 않았습니다. 같은 주제의 열린 중복 PR은 지금 목록에서 보이지 않습니다.

라인 src/responses/code-mode-shell-input.ts parseCodeModeShellInput - 명령 글자가 Bun 자바스크립트 문법으로 읽히면 셸로 안 바꿉니다. 의도한 안전장치이지만, 흔한 셸이 자바스크립트처럼 보이는 경우가 있습니다. 예: ls, pwd, ls -la(빼기 식), cd /tmp && ls(경로가 나누기·정규식처럼 읽힘), rm -rf /tmp/x. 이런 {"cmd":"..."} 는 예전처럼 그대로 남아 실패할 수 있습니다. PR이 고친 StepFun 예(cd "/tmp/..." && git status --short)는 따옴표·공백 때문에 잘 변환됩니다. 짧은 명령·플래그·경로가 많은 실제 호출은 아직 구멍입니다.

라인 src/responses/code-mode-shell-input.ts mayBecomeCodeModeShellInput - 입력이 {로 시작하면 미리보기를 끝까지 막습니다. code-mode exec에 JSON이 아닌 다른 {...}가 오면, 끝까지 진행형 미리보기가 비었다가 완료 때 한꺼번에 나갑니다. 셸이 아니면 내용은 그대로 나가야 하고, 테스트도 일반 자바스크립트 진행형은 지켜 줍니다. 동작은 맞지만, {로 시작하는 드문 자바스크립트 미리보기는 늦어질 수 있습니다.

메인테이너의 판단이 필요한 지점
“자바스크립트로 읽히면 절대 셸로 추측하지 않는다”를 지금처럼 넓게 둘지, tools./text(/await처럼 code-mode 티가 날 때만 자바스크립트로 볼지 정해 주세요. 넓게 두면 ls -la 같은 짧은 셸 객체는 계속 깨질 수 있습니다. 좁히면 그 구멍은 줄어들지만, 예전에 cmd 필드에 진짜 자바스크립트를 넣던 경로를 잘못 건드릴 위험이 있습니다. draft라서 라이브 StepFun 확인을 머지 전에 할지, 포커스 테스트만으로 올릴지도 선택이 필요합니다.

너의 추천
관측된 StepFun 깨짐과 문서·테스트 범위는 이대로 dev에 가져가도 됩니다. 다만 ls -la / cd /tmp && ls 류가 자바스크립트로 오인되는 구멍은 알고 머지하거나, 후속 PR로 “단일 식별자만 모호 처리, /·단항 - 플래그 패턴은 셸로 본다”처럼 규칙을 조금 좁히는 편이 좋습니다. 429 재시도·프리뷰 배포 이야기는 이 PR 범위 밖입니다. 무효·중복으로 닫을 다른 열린 PR은 보이지 않습니다.

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

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

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

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@lidge-jun

Copy link
Copy Markdown
Owner

Closing as superseded. The changes from this PR (head 9430bbdad79e, by @Flowershangfromthebranches) were carried with author credit into #5542, which was consolidated into #5608. #5608 merged to dev as 9579bbe. The carry was cherry-picked or reworked, not merged, so this branch's own commit history is not part of dev. I compared this head against current dev and found its behavior and tests present. That covers structured shell payload compilation for code-mode exec.

This fix is on dev only. It is not in the stable v2.63.0 release and will ship in a later release. Thank you for the contribution.

@lidge-jun lidge-jun closed this Sep 23, 2026
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.

2 participants