Conversation
…forwarded role (lidge-jun#5334 follow-up) lidge-jun#5334 made the developer wire role tri-state: an undeclared destination folds it to system. Two suites asserting role:"developer" on the Chat wire were missed because they are about tool-result repair ordering and document parts, not role selection — declare the destination, per the convention the change established. Verified: both files fail on dev@600075d2 with system-for-developer wire roles and pass with the declaration. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: luvs01/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "session_id", | ||
| "session-id", | ||
| "thread-id", | ||
| "user-agent", |
There was a problem hiding this comment.
🔍 Explicit security review is required
This changes the shared header set used by Codex authentication materialization. Repository policy requires explicit security review for authentication-boundary changes before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — this touches the shared allowlist consumed by auth materialization, so it needs the explicit security review per MAINTAINERS.md before merge. Leaving this thread open for the maintainer; the fix commit narrows the semantics so user-agent is fallback-only wherever it overlays configured headers.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…user-agent-fallback-issue Co-Authored-By: Epinephrine <luvs01@hanmail.net> # Conflicts: # tests/adapters/openai/openai-chat-dangling-toolcalls.test.ts # tests/responses/chat-inline-document-bytes.test.ts
…ed caller value A caller User-Agent now reaches the canonical forward path through auth materialization, but the generic FORWARD_HEADERS overlay wrote it after provider.headers, replacing or duplicating a configured value. Apply it through applyCallerUserAgentFallback only, shared by the web-search and vision sidecar replays, so a configured provider header wins case-insensitively and the caller fingerprint fills only the gap. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…d relays The standalone search, images, live, and context-history relays overlay materialized auth headers after configured provider headers, so the caller User-Agent newly retained by FORWARD_HEADERS replaced the configured fingerprint there too. Skip the name in each overlay and defer to applyCallerUserAgentFallback, which now also accepts a Headers target for the context relay. Sidecar tests cover configured precedence and caller fallback; a wiring guard pins all four relays to the shared helper. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nd audio sends Compact's FORWARD_HEADERS overlays and the audio upstream both write the materialized caller header set without ever merging provider.headers, so the allowlist addition sent the caller fingerprint even when the provider config carries a User-Agent of its own. Extend the shared fallback with a provider header oracle so those paths keep the configured value; the caller User-Agent still fills the name when the provider leaves it unset. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The native compact fallback only ran while materializing a Codex account, so a canonical API-key provider sent neither the configured nor the caller User-Agent. Apply the fallback after the auth-mode branch against the final compactProvider so every compact send gets the same precedence. The record branch of applyCallerUserAgentFallback also returned early on an existing User-Agent even when a configured provider value was supplied; it now evicts stale duplicates and writes the configured value, matching the Headers shape. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
1 participant
Motivation
Headersset fromFORWARD_HEADERSwhich omittedUser-Agent, so the new adapter-level fallback could not see the caller fingerprint in real HTTP/Chat/WebSocket flows.User-Agentas a single non-credential fallback while keeping provider-configured headers authoritative.Description
Codex Task