fix: pin Union Alpha wire and namespace Go sessions - #4930
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe change makes OpenCode Go session values protocol-specific, preserves destination recognition after adapter overrides, reorders wire resolution before transport resolution, and pins ChangesOpenCode Go session routing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Request
participant RouteResolver
participant OpenCodeGoTransport
participant SessionHeader
Request->>RouteResolver: provide routed provider and model
RouteResolver->>RouteResolver: resolve wire protocol override
RouteResolver->>OpenCodeGoTransport: pass settled provider and routed destination
OpenCodeGoTransport->>SessionHeader: derive protocol-specific session value
Merge Risk: ⚪ Minimal · up to The routing, session namespacing, and wire-pinning changes have no established merge-blocking risk in the supplied evidence. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
리뷰 · 우선순위 75 / 80이 PR은 OpenCode Go의 그런데 Anthropic으로 어댑터를 뒤집으면, 예전에 세션 헤더 라인 tests/providers/opencode-go-session-header.test.ts (~315–370) - 라인 src/providers/opencode-go-transport.ts ( 경로 #4847 vs 이 PR - 이 패치는 wire pin과 session namespace까지만 간다. PR 본문도 Messages 503이 남는다고 적었다. 이슈를 닫으면 “Union Alpha가 된다”로 오해된다. Refs 유지를 권한다. 경로 CI - 헤드 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4207419ff3
ℹ️ 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".
| /** Derive a provider- and wire-scoped opaque value without exposing Codex task or subagent ids. */ | ||
| export function deriveOpenCodeGoSessionId(sessionLane: string, wireProtocol: string): string { |
There was a problem hiding this comment.
This changes behavior in both src/providers/ and src/server/, but updates only structure/transports/responses.md. structure/INDEX.md maps those source areas to several additional contract documents, including runtime.md, subagents.md, and transports/inventory.md; synchronize every mapped document in this change, or correct the ownership map if those documents no longer describe these areas, so the maintained architecture contracts do not drift.
AGENTS.md reference: src/AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
The Claude metadata lane is not the hashed metadata.user_id. claude-messages.ts turns prompt_cache_key into a UUID through uuidFromHex and normalizes THAT, so the lane is sha256(uuidFromHex(sha256(user_id)[:32]))[:32]. The three metadata vectors were computed without that step. Derived independently of the production helpers and validated against the unchanged v1 values these tests carried before the namespace change: the same reconstruction reproduces both of the previous metadata vectors exactly, which is what shows the lane is right rather than merely self-consistent. The other eight vectors were already correct and are untouched.
The loop lists CHAT_SESSION_VECTORS for glm-5.2 and RESPONSES_SESSION_VECTORS for the Muse model but destructured only [model, url], so every case in it read whichever sessionVectors binding was in scope instead of its own. Muse settles on the Responses wire, and these vectors are now protocol-scoped, so each model needs the table the loop names for it.
1 participant
Summary
union-alphamodel id to the Anthropic wire, based on the evidence in [Provider]: Union Alpha inference fails on OpenCode routes; per-model Anthropic protocol mismatch #4847, while leaving sibling Chat and Responses models unchanged.x-opencode-sessionvalues by the final wire protocol with a versioned hash separator. Sessions remain stable for the same lane and protocol, and operator-supplied headers still win unchanged.Endpoint is unavailablewith a session header. This change does not establish successful Union Alpha inference; it fixes protocol selection and session namespacing only.Refs #4847
Verification
git diff --checksrc/server/responses/core-normalize.ts,src/server/chat-completions.ts,src/providers/opencode-go-transport.ts, andsrc/server/adapter-resolve.ts.tests/providers/opencode-go-session-header.test.ts.resolveOpenCodeGoTransportcall supplies the original routed destination separately from the settled provider.ocxbinary were not run because this lane explicitly forbids local verification. Hosted CI remains the executable verification.Checklist
Summary by CodeRabbit
New Features
union-alphaconsistently uses the Anthropic-compatible wire protocol.Bug Fixes
union-alpharequests across supported entry points.Documentation
Known one-time effect: existing Go sessions rotate once
The derivation prefix moves from
opencodex/opencode-go/session/v1to.../v2, so every generatedx-opencode-sessionvalue changes on upgrade. A conversation that was in flight across the upgradeis seen by OpenCode Go as a new session once, which costs that conversation its routing affinity for
one turn. Sessions are stable again from that point, and an operator-supplied session header is never
rewritten.
The version bump is deliberate rather than avoidable: the hash input genuinely changed, and reusing
v1would have produced two different derivations behind one version label. The alternative —namespacing only the Anthropic wire and leaving Chat and Responses on the v1 value — would have kept
existing sessions intact, but it makes the derivation depend on which protocol you happen to be on
and leaves the versioned prefix lying about its input.