Skip to content

fix(codebuddy): refuse DSML scaffold with a bare tool name - #4887

Merged
lidge-jun merged 1 commit into
devfrom
codex/codebuddy-scaffold-bare-tool-names
Sep 17, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/codebuddy-scaffold-bare-tool-names

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

  • The 2.57.0 CodeBuddy scaffold guard only refuses a DSML block whose invoke target carries the functions. namespace prefix. The routed model writes the bare name, so every marker misses and the whole control block is forwarded as assistant text — which is why [codebuddy] Routed tool-call markup reaches the client as assistant text (no scaffolding guard, unlike #4190) #4596 still reproduces on the released build.
  • [codebuddy] 2.57.0 scaffold guard misses bare tool names (DSML_INVOKE_PREFIX requires the functions. prefix) - #4596 still reproduces #4852 measured this against the shipped filter class: functions.exec, functions.Bash and functions.apply_patch are refused, while Bash, exec, shell and apply_patch all leak. The block opener matches in every case and the delta-boundary handling already works; the namespace prefix on the invoke target was the only gap.
  • DSML_INVOKE_PREFIX now stops at the opening quote and refusal requires a non-empty name character after it. The narrowness that keeps prose safe is the two-line calls-plus-invoke grammar at column zero outside fenced Markdown, not the namespace, and that is unchanged. So are the held-suffix bound and the single vendor_scaffold_detected outcome.
  • Scope stays a refusal guard. This does not turn vendor text into executable tool calls.

Closes #4852

Verification

  • Local suite, typecheck and build: not run (explicit restriction for this lane; a local suite has previously deleted real ~/.opencodex data). Hosted CI on this head is the verifier.
  • Static review of the shortened constant against both partial-match paths: a chunk ending inside the prefix still holds via prefixAtEnd / DSML_INVOKE_PREFIX.startsWith(invokeRest), and refusal fires once the first name character arrives.
  • New cases in tests/providers/codebuddy-adapter.test.ts: bare Bash, exec, shell and apply_patch refused; the functions.-prefixed [codebuddy] Routed tool-call markup reaches the client as assistant text (no scaffolding guard, unlike #4190) #4596 samples including the split-delta one still refused; a bare-name block split at the invoke prefix refused; quoted prose, fenced examples, an empty name, and a calls line with no invoke line all still forwarded.

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.

Summary by CodeRabbit

  • Bug Fixes

    • Improved CodeBuddy compatibility by detecting a broader range of tool-invocation scaffolding, including bare and namespaced tool names.
    • Prevented incomplete split invocations from being processed prematurely while preserving invocations with empty names.
  • Tests

    • Added coverage for common tools such as Bash, exec, shell, and apply_patch, including streamed and empty-name scenarios.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 17, 2026 08:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-17T09:00:53.098430Z 51e80da PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7ece85e8-ef74-4ee6-98cb-9017e5084471

📥 Commits

Reviewing files that changed from the base of the PR and between a1fe84b and 51e80da.

📒 Files selected for processing (3)
  • src/adapters/codebuddy/scaffold-guard.ts
  • structure/providers/chat-compat.md
  • tests/providers/codebuddy-adapter.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The CodeBuddy scaffold guard now detects bare and namespaced DSML invoke names instead of requiring the functions. prefix. Documentation and tests cover split prefixes, empty names, and literal output handling.

Changes

CodeBuddy scaffold guard

Layer / File(s) Summary
Broaden invoke detection
src/adapters/codebuddy/scaffold-guard.ts
The guard accepts any non-empty invoke name after the DSML prefix. It still rejects whitespace and quote characters as the first name character.
Document and test named invokes
structure/providers/chat-compat.md, tests/providers/codebuddy-adapter.test.ts
Documentation now describes named bare or namespaced invokes. Tests cover Bash, exec, shell, apply_patch, split prefixes, empty names, and unchanged quoted or fenced literals.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 51e80

The guard now refuses named bare and namespaced scaffolds while preserving documented safe pass-through cases. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: the CodeBuddy guard now refuses DSML scaffolding that uses a bare tool name. It is concise, specific, and directly related to the implementation and te…
Linked Issues check ✅ Passed The changes satisfy issue #4852. In src/adapters/codebuddy/scaffold-guard.ts, DSML_INVOKE_PREFIX now ends at name=", and the scan accepts a non-empty first tool-name character while rejecting wh…
Out of Scope Changes check ✅ Passed The changed production code, regression tests, and compatibility documentation all directly support issue #4852. The tests validate the requested bare-name behavior and protect existing split-delta an…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/codebuddy-scaffold-bare-tool-names

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 added the bug Something isn't working label Sep 17, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 77 / 80

이 PR은 CodeBuddy 어댑터의 DSML 스캐폴드 거부가 빈 도구 이름 다음 글자까지 보도록 고칩니다. 현재 dev(HEAD a1fe84b31, 패키지 2.58.0, tip #4880 L4 Responses 계획 문서)의 src/adapters/codebuddy/scaffold-guard.ts는 여전히 DSML_INVOKE_PREFIX<||dsml|| invoke name="functions.까지로 잡습니다. 2.57.0에 들어간 가드(#4776)도 같은 형태였고, #4596에서 보고된 “벤더 제어 블록이 어시스턴트 텍스트로 새는” 문제는 그 가드가 실서비스에 있어도 재현됩니다. 이유는 간단합니다. 라우트된 모델이 쓰는 invoke는 functions.Bash가 아니라 Bash, exec, shell, apply_patch처럼 네임스페이스 없는 이름이기 때문입니다. functions.만 기다리는 상수는 마커를 전부 놓치고, calls+invoke 블록 전체가 그대로 내려갑니다. #4852가 그 측정을 이슈로 고정했고, 이 PR은 그 이슈를 닫습니다.

고치는 폭은 의도적으로 좁습니다. DSML_INVOKE_PREFIX를 여는 따옴표까지만 줄이고, 거부 조건에 “따옴표 바로 다음이 비어 있지 않고 공백/따옴표가 아닌 한 글자”를 추가합니다. 그래서 name=""는 그대로 통과하고, name="Bash"나 예전 name="functions.exec"는 둘 다 거부됩니다. 산문·인용·펜스·들여쓰기 안전을 지키는 쪽은 예전과 같습니다. 여전히 0열의 인용되지 않은 calls 줄 + 바로 다음 줄 invoke 문법만 보며, held-suffix / prefixAtEnd / 한 번의 vendor_scaffold_detected 결과도 그대로입니다. 벤더 텍스트를 실행 가능한 툴 콜로 승격하지 않습니다. 거부 가드만입니다. structure/providers/chat-compat.mdfunctions.*만 적던 문장을 “bare or namespaced named invoke”로 맞춰 두어 문서 드리프트를 막았습니다.

테스트도 같은 이야기를 코드로 고정합니다. tests/providers/codebuddy-adapter.test.ts에 bare Bash/exec/shell/apply_patch 거부, invoke 접두가 델타로 갈라진 bare-name 거부, 빈 이름 전달, 그리고 인용·펜스 예시가 bare 이름으로도 여전히 통과하는지까지 넣었습니다. 기존 functions. 샘플(스플릿 델타 포함)도 계속 거부됩니다. 로컬 suite/typecheck/build는 이 레인의 명시 제한으로 돌리지 않았고, hosted CI(exact head 51e80da)가 검증기입니다. types.ts/config.ts 분할과 무관하고 중복 PR도 아닙니다. base는 dev tip과 맞고 ready PR입니다. mergeable_state는 지금 blocked(CI 대기)입니다.

라인 11 - DSML_INVOKE_PREFIX를 여는 따옴표까지만 남겨, bare 도구 이름도 같은 접두 경로로 잡히게 한다. functions.만 기다리던 2.57.0 구멍의 직접 수정이다.

라인 80-81 - invokeRest가 접두와 일치한 뒤, 첫 이름 글자가 있고 공백/따옴표가 아닐 때만 fail한다. 빈 name=""와 미완성 접두는 거부하지 않고 hold/forward한다.

tests/providers/codebuddy-adapter.test.ts (bare %s / split prefix / empty name) - #4852가 측정한 누수 이름 네 개와 델타 경계, 빈 이름 통과를 고정한다. 회귀가 다시 열리면 바로 빨개진다.

structure/providers/chat-compat.md - 계약 문장을 bare/namespaced named invoke로 고쳐 구현과 문서를 맞춘다. 가드만 바뀌고 문서가 남는 드리프트를 피한다.

거절 범위 / 승격 없음 - 스코프가 거부 가드에 머문다. 벤더 DSML을 Codex 툴 콜로 바꾸지 않으므로 실행 권한 경계는 그대로다.

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

  • exact-head hosted CI(특히 hygiene + codebuddy 관련 테스트 레인)가 그린일 때만 merge할지. 로컬 검증은 레인 제한으로 비어 있다.
  • 이 픽스를 2.58.0 release train에 바로 태울지, 아니면 2.57.x 핫픽스/체리픽 후보로도 볼지(#4596은 출시본에서도 재현).
  • bare 이름 거부가 넓어진 뒤, 실제로 0열 calls+invoke 문법을 쓰는 “설명용” 응답이 있는지 한 번만 더 볼지(설계상 그 문법은 스캐폴드로 본다).

너의 추천
KEEP — tip a1fe84b31 기준 exact-head CI가 그린이면 merge. #4852/#4596 누수의 원인·수정·테스트·문서가 한 줄로 맞고, 실행 승격 없이 가드만 고친 점이 안전하다. CI 끝나기 전 merge는 하지 말 것.

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

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51e80da264

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// discussing one tag.
const DSML_CALLS_LINE = "<||dsml|| calls>";
const DSML_INVOKE_PREFIX = "<||dsml|| invoke name=\"functions.";
const DSML_INVOKE_PREFIX = "<||dsml|| invoke name=\"";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update every mapped adapter document

This changes src/adapters/, but the commit updates only structure/providers/chat-compat.md; the source map also lists runtime.md, transports/byte-accounting.md, transports/responses.md, transports/inventory.md, data-planes/inbound-compat.md, providers/cursor.md, and adapters/registry.md. Update every mapped document in this change as required, so the architecture set does not describe the adapter area inconsistently.

AGENTS.md reference: src/AGENTS.md:L11-L11

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun
lidge-jun merged commit ccf3b03 into dev Sep 17, 2026
30 checks passed
@lidge-jun
lidge-jun deleted the codex/codebuddy-scaffold-bare-tool-names branch September 17, 2026 10:27
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