Skip to content

fix(opencode-go): bound replayed tool promotion - #5235

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:fix/opencode-go-replay-tools-bound
Sep 20, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:fix/opencode-go-replay-tools-bound

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Bound replayed additional_tools promotion in the OpenCode Go Responses path. normalizeOpenCodeGoAdditionalTools promoted every additional_tools input wrapper, including wrappers replayed from continuation history, so stale catalogs were re-declared as if user-supplied for the current turn. The function now takes the replayed-input prefix length and removes historical wrappers without promoting their catalogs; passthrough.ts passes the already-parsed _replayPrefixLen through.

Verification

  • bun test tests/providers/opencode-go-grok46-responses.test.ts — 39 pass, 0 fail (includes a new regression test covering a replayed wrapper before the current-turn boundary).
  • bun x tsc --noEmit — clean.

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.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed continuation requests so tools restored from conversation history are not incorrectly promoted into the active tool list.
    • Preserved tools added during the current turn while excluding historical tool wrappers from outgoing request data.
    • Added coverage to verify correct handling of replayed conversation content.

@coderabbitai

coderabbitai Bot commented Sep 20, 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: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b8ec7edc-a9e2-49af-9de6-f0f04540ecb4

📥 Commits

Reviewing files that changed from the base of the PR and between 2a2c01a and 609aef3.

📒 Files selected for processing (3)
  • src/adapters/openai-responses/passthrough.ts
  • src/adapters/opencode-go-additional-tools.ts
  • tests/providers/opencode-go-grok46-responses.test.ts

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


📝 Walkthrough

Walkthrough

The request path now passes replay-prefix length to additional-tool normalization. Historical additional_tools wrappers are removed without promotion, while current-turn wrappers retain their existing promotion behavior. Tests cover continuation-history requests.

Changes

Replay-aware additional-tool normalization

Layer / File(s) Summary
Replay-aware normalizer
src/adapters/opencode-go-additional-tools.ts
normalizeOpenCodeGoAdditionalTools accepts an optional replay-prefix length. Historical wrappers are removed without promoting their tools. Wrappers at or after the current-turn boundary are still promoted.
Request wiring and validation
src/adapters/openai-responses/passthrough.ts, tests/providers/opencode-go-grok46-responses.test.ts
The passthrough adapter forwards _replayPrefixLen. Tests verify that historical tools are absent from both input and the outgoing tools field.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant buildRequest
  participant PassthroughAdapter
  participant ToolNormalizer
  buildRequest->>PassthroughAdapter: Pass _replayPrefixLen
  PassthroughAdapter->>ToolNormalizer: Pass replayPrefixLength
  ToolNormalizer-->>PassthroughAdapter: Remove history wrappers
  ToolNormalizer-->>PassthroughAdapter: Promote current-turn tools
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing incorrect promotion of replayed tools in the OpenCode Go path.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

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

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

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.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 68 / 80

이 PR은 OpenCode Go로 Responses를 보낼 때, 이어하기(continuation) 히스토리에 남아 있던 additional_tools 포장을 이번 턴 도구처럼 올려 버리던 버그를 막습니다. 예전 normalizeOpenCodeGoAdditionalTools는 input 안의 그 포장을 전부 풀어서 tools에 붙였습니다. 그래서 예전에 쓰던 목록이 다시 “지금 사용자가 준 것”처럼 선언됐습니다. 지금은 _replayPrefixLen(이미 파서가 잡는 히스토리 앞부분 길이)을 받아서, 그 앞쪽에 있는 포장은 목록을 올리지 않고 포장만 지웁니다. Go는 이 포장 자체를 받지 못하니 지우는 쪽은 맞습니다. passthrough.ts는 이미 쓰던 parsed._replayPrefixLen을 그대로 넘깁니다. xAI 쪽 injectXaiResponsesXSearch가 쓰는 “현재 턴만 본다” 경계와 같은 생각입니다.

베이스는 dev입니다. 이 글을 쓰는 지금 dev 끝은 2a2c01a7102e입니다. #5224입니다. 이 브랜치는 그 끝보다 커밋 6개가 뒤입니다. 겹치는 파일은 passthrough.ts 등 어댑터 쪽이 있을 수 있으니 Ready 전에 rebase가 필요합니다. types.ts와 config.ts는 안 바꿉니다. 고친 파일은 opencode-go-additional-tools.ts, passthrough.ts, 회귀 테스트 하나뿐입니다. 상태는 드래프트이고, 본문 준비 체크는 0/4입니다. hygiene는 통과입니다. 작성자가 적은 bun test(39통과)와 tsc는 여기서 다시 돌리지 않았습니다.

라인 - tests/providers/opencode-go-grok46-responses.test.ts:206 - 회귀는 “히스토리 포장만 있고, 이번 턴 포장은 없음”만 봅니다. 앞쪽(히스토리) 포장은 버리고, 뒤쪽(이번 턴) 포장은 올려야 하는 섞인 입력은 테스트가 없습니다. 함수 경계(index < currentTurnStart)는 그 경우를 의도한 것으로 보이지만, 증명 테스트가 없습니다.

라인 - src/adapters/opencode-go-additional-tools.ts:36 - 히스토리 포장은 목록을 올리지 않고 삭제만 합니다. 이어하기가 필요한 도구를 이미 맨 위 tools나 이번 턴 포장으로 다시 넣지 않으면, 이번 요청은 도구 없이 나갈 수 있습니다. 버그 설명(낡은 목록을 이번 턴 권한으로 올리면 안 됨)과는 맞지만, 이어하기 경로가 도구를 어디서 복구하는지와 맞춰 봐야 합니다.

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

히스토리에만 있던 additional_tools를 올리지 않는 것이 맞는 제품 규칙인지 확인해 주세요. 이어하기가 맨 위 tools로 이미 복구한다면 지금 동작이 맞습니다. 복구가 이 포장에만 의존한다면, 포장을 지우는 것만으로는 도구가 빠집니다.

너의 추천

방향은 맞습니다. 범위가 작고, _replayPrefixLen을 넘기는 방식도 기존 패턴과 같습니다. tip(2a2c01a7, 커밋 6개) 위로 rebase하고, 준비 체크를 채운 뒤 Ready로 올리세요. 가능하면 히스토리 포장 + 이번 턴 포장이 같이 있는 테스트 하나를 더 넣으세요. types.ts/config.ts 분할과 무관해서 이 PR을 닫지 마세요.

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

@luvs01
luvs01 marked this pull request as ready for review September 20, 2026 01:51
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@lidge-jun
lidge-jun merged commit 9164338 into lidge-jun:dev Sep 20, 2026
37 of 39 checks passed

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved on exact head 609aef3f72428fcb0a2def11105fb7539866b96c. The replay-prefix boundary removes historical additional_tools wrappers without treating history as current tool authority, while wrappers appended in the current turn remain eligible for promotion. Current dev is the PR base, there are no unresolved review threads, and exact-head Cross-platform CI plus the latest React Doctor run are green. A mixed historical/current-wrapper regression would still be useful follow-up coverage, but the current implementation is bounded and mergeable.

@luvs01
luvs01 deleted the fix/opencode-go-replay-tools-bound branch September 20, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants