feat(openai): OpenAI-compatible API adapter for Open WebUI and SDK clients - #614
Merged
Conversation
First phase of the /v1 OpenAI-compatible adapter (Open WebUI integration).
Wire-format records plus the eddi.openai-compat.* configuration surface; no
behaviour yet.
Two shapes are deliberately loose rather than strict:
- ChatCompletionRequest.user is a JsonNode, not a String. The OpenAI spec says
string, but Open WebUI sends an object {name,id,email,role} for pipeline
models, which would fail a String binding.
- ChatMessage.content is a JsonNode. It is polymorphic in the spec (string for
text, array for multimodal); a JsonNode handles both without a custom
deserializer.
Unknown request fields are ignored, not rejected: clients always send
temperature/max_tokens/tools, and those belong to the agent's langchain.json.
The /v1 permission entry is required, not optional. The catch-all
authenticated policy on /,/* would otherwise capture /v1/* and reject the
shared-API-key bearer token at the OIDC layer, before the adapter's own filter
could run.
Adapter defaults to disabled. Also adds the full implementation plan.
Refs planning/openai-api-adapter-plan.md
…ication Model catalogue, resolution, the /v1 REST surface, and the two auth pieces. Chat completions land in the next commits. Model ids are <slug>-<last 6 of agentId>, not the bare slug: descriptor names are not unique, so a bare slug would make resolution non-deterministic the moment two agents are both called "Support". Name and slug lookups are still accepted for convenience but only when they match exactly one agent -- an ambiguous match raises rather than guesses, because silently routing a conversation to the wrong agent looks like an agent bug, not a routing bug. Slugging folds accents (NFD + strip combining marks) instead of dropping them. Without that, "Ubersicht" slugged to "bersicht" and every non-ASCII agent name became unrecognisable in the model dropdown. Caught by its own test. The catalogue is read from IAgentFactory + IDocumentDescriptorStore, NOT from IRestAgentAdministration. That interface carries a type-level @RolesAllowed({"eddi-admin","eddi-editor"}) which Quarkus enforces via a CDI interceptor, so injecting it here would 403 for every ordinary caller. REST facades are the authorization boundary; a public surface must not reach around one. Authentication has two modes because Open WebUI sends an opaque sk-... secret that the OIDC mechanism rejects as a malformed JWT before application code runs. Under http-policy=permit the filter enforces the shared key itself (constant-time compare); under 'authenticated' Quarkus OIDC has already run and the filter only reads the identity. A caller whose identity cannot be resolved is refused rather than served, since serving it would merge every such caller into one shared conversation and therefore one shared memory. OpenAiStartupGuard refuses to boot on enabled + unauthenticated + authorization on, naming the three ways to fix it. Errors use the OpenAI {"error":{...}} envelope throughout; the mapper is scoped to one exception type so it cannot alter the rest of EDDI's REST behaviour. Tests: 43. Mutation-checked -- reverting the ambiguity guard and the identity refusal each makes the relevant tests fail.
Only the last user message is carried across. The client resends the whole
conversation every turn because the OpenAI protocol is stateless; EDDI keeps
its own memory, so replaying that history would double every turn.
The last system message becomes an openai_system_message context entry rather
than overriding the agent's configured prompt. A client that could rewrite the
system prompt would make the agent non-portable, so agent designers opt in via
{context.openai_system_message}. Open WebUI's RAG chunks arrive here too.
Images map to attachment_N context entries in the shape
AttachmentContextExtractor already documents, so they flow through the existing
forwarder with its vision gating, byte caps and SSRF-guarded fetching. No
adapter-side image handling at all.
Two parsing details that would otherwise bite:
- 'data:image/png,payload' is legal and contains no ';'. Deriving the MIME by
scanning to ';' throws StringIndexOutOfBounds on it. Parsed comma-first
instead, and a data URI with no comma is skipped rather than failing the turn.
- Remote URLs get a concrete MIME derived from the file extension, never
'image/*'. A wildcard passes the forwarder's startsWith("image/") gate but is
then handed verbatim to ImageContent.from(base64, mimeType), where providers
reject it.
Tests: 24, driven by parsing real OpenAI JSON rather than constructing records,
so the polymorphic content/user fields go through the real binding path.
The bridge, the SSE writer, and POST /v1/chat/completions. ONE method serves both sync and streaming, dispatching on the 'stream' field of the body. Content negotiation cannot work here: openai-python hardcodes Accept: application/json and never varies it by stream, and Open WebUI sends no Accept header at all (it sniffs the response content-type instead). Two @produces methods would route every streaming request to the JSON one. The body is the only reliable signal, and it is what the OpenAI spec specifies. Conversations are keyed by (channel:openai:<agentId>:<chatKey>, userId) in IUserConversationStore, mirroring Slack. chatKey comes from X-OpenWebUI-Chat-Id -- the header Open WebUI forwards under ENABLE_FORWARD_USER_INFO_HEADERS -- falling back to the OpenAI 'user' field. That is what gives each chat window its own conversation; without it every window a user opens against one agent shares a single memory. There is deliberately NO new-chat heuristic. Inferring a restart from the message count is unreliable (regenerate and edit-and-resend look identical to a first message) and destructive, since ending a conversation discards its memory irrecoverably. A new chat is a new chat key, which is a new intent. HITL is surfaced as chat text, never as an HTTP error -- a 4xx makes clients discard the user's message. onSkipped is split by snapshot state via sentinels (as Slack does): AWAITING_HUMAN yields a 'reviewer must decide' message, while busy/ended yields 429 so clients back off. A turn that completes while paused returns its output plus the notice and the conversation id. Streaming reconciles tokens against the snapshot: rule-based agents emit text only at onComplete, so the snapshot text is sent only when no token arrived -- otherwise the whole reply would appear twice. Other decisions: a conversation that ended mid-turn is retried exactly once on a fresh one (bounded, so a broken agent cannot spin); a lost createUserConversation race adopts the winner and ends the orphan; stateless turns always end their conversation, including on failure; in-flight completions are bounded by a semaphore because each holds a worker thread. Tests: 41 more (114 total). Mutation-checked: collapsing the onSkipped sentinel distinction makes the HITL tests fail.
docs/open-webui-integration.md covers setup, the stateful/stateless bridge, configuration, the security model, the Open WebUI settings that matter, supported features, error contract, troubleshooting and known gaps. Two settings are called out as load-bearing because getting them wrong fails quietly rather than loudly: ENABLE_FORWARD_USER_INFO_HEADERS (without it every chat window a user opens against one agent shares a single conversation and its memory), and the title/tag generation model (pointed at a normal EDDI model, Open WebUI's 'write a title' prompt becomes a real turn in the user's conversation). The trust-user-headers and allow-anonymous trade-offs are documented as warnings rather than buried in a table -- both are deliberate delegations with real consequences if misread. Changelog records the false premises the four earlier plan drafts rested on and how each was verified, since the corrections are what shaped the design. Plan document keeps its pre-implementation form with a short delta section for the four things that differed in practice.
Most serious: a semaphore permit leak on the streaming path. The permit was
handed to the StreamingOutput body to release, but if that body never runs --
a client disconnecting before serialization starts, say -- it is never
reclaimed. After max-concurrent-requests such events the adapter returns 429
permanently, until restart, with nothing in the logs to explain it. The
resource now releases unconditionally and the stream body acquires its own
permit inside a single try/finally, so neither can be orphaned. A stream that
cannot get a slot degrades to an in-band busy notice, since the 200 is already
committed by then.
GET /v1/models/{id} echoed back whatever the caller typed instead of the
canonical id, so a lookup by agent name returned an id absent from
GET /v1/models -- a client round-tripping the answer would then ask for a model
that does not exist. ResolvedModel now carries requested and canonical ids
separately, plus the descriptor timestamp that was hardcoded to 0.
InterruptedException was swallowed while waiting for a turn, leaving the worker
thread looking healthy with its shutdown signal gone.
A null exception message reached the user as the literal text "null" in the
stream error path; NPEs carry no message, so this was the likely case.
hasSentContent() actually reported whether the stream had started -- true even
after a content-free finish(). Renamed hasStarted().
The eddi.openai.requests counter the plan documented was never implemented.
Added with mode/outcome tags so 'paused' (reviewers behind) and 'busy' (clients
racing) are distinguishable from real errors.
Verified rather than assumed: this project sets
enable-reflection-free-serializers=false, so @JsonProperty on record components
works and finish_reason/owned_by serialize correctly. Added OpenAiWireFormatTest
to pin it -- the non-streaming response shape had no coverage at all.
Corrected the docs rather than the code on one point: unimplemented /v1 paths
return Quarkus' plain 404, not an OpenAI envelope. A catch-all route would risk
shadowing the real endpoints for a cosmetic gain.
Tests: 119 (was 104). Mutation-checked the two new behavioural fixes -- removing
the token-reconciliation guard and bypassing the null-message handling each make
the relevant tests fail.
Adds the file and input_audio content-part types alongside image_url. All three map to attachment_N context entries, so EDDI's existing forwarder does the real work -- capability gating, byte caps, PDF text extraction, SSRF-guarded fetching. The adapter still handles no media itself. PDFs now reach PdfFileContent when the model has native document support, and are text-extracted and inlined when it does not. Any type the text extractor already handles (.txt, .md, .csv, .json, .xml, .html) is inlined too. The three wire formats are inconsistent in ways that are easy to get wrong, so each is verified against the OpenAI docs and pinned by a test: - input_audio.data is RAW base64 with no data: prefix -- unlike every other binary payload in this protocol -- with the type in a separate 'format' field. And mp3 maps to audio/mpeg: "audio/" + format would yield audio/mp3, which is not a real media type and which providers reject. - file.file_data IS a full data URI. The declared filename beats a generic application/octet-stream type, because clients that base64 a file without sniffing it send exactly that -- so contract.pdf still reaches the PDF path. - file.file_id references the OpenAI Files API, which EDDI does not implement. Those parts are skipped with a warning naming the fix, rather than becoming empty attachments. filename is also accepted as file_name, which appears in the wild. The per-turn cap now counts all three types together rather than images alone. Note this path is for the OpenAI SDK and similar clients: Open WebUI does not send documents as file parts, it runs its own RAG over uploads and injects the retrieved text into the system message. Tests: 132 (was 119). Mutation-checked -- naive audio MIME concatenation and dropping the filename-over-generic-type rule each make their tests fail.
…model suffix
The :stateless model suffix exists for a specific reason: a model name is the
only per-request dimension a UI like Open WebUI can express. Its
title-generation setting is a dropdown, so a query param, header or body field
could not be selected there at all. That makes the suffix necessary, but it
does mean behaviour is encoded in an identifier, which is a smell when it is
the ONLY way to ask.
So the same thing is now expressible as a body field for callers that can say
what they mean:
{"model": "support-a3f9c1", "messages": [...], "stateless": true}
client.chat.completions.create(..., extra_body={"stateless": True})
The two are OR-ed rather than letting either win. model:"x:stateless" together
with stateless:false is self-contradictory, and of the two readings, running
stateless merely loses continuity while running stateful would persist a
conversation the caller may not have wanted -- so the flags fail toward the
less surprising outcome.
expose-stateless-variants=false blocks BOTH routes (400 invalid_request_field).
Gating only the suffix would have left the kill switch circumventable by moving
the request out of the model id and into the body.
ResolvedModel.asStateless() also normalises canonicalModelId, so both routes
describe themselves identically rather than one reporting a stateful id.
Docs now also correct two things I had overstated about :stateless: it is not
"no memory at all" (user-scoped long-term memory is keyed by userId, not by
conversation, so it still loads and persists for agents that enable it), and it
is not "classic OpenAI semantics" (only the last user message is sent in both
modes, so it is genuinely single-turn rather than client-managed history).
Tests: 143 (was 132), including the first RestOpenAiAdapterTest -- a plain unit
test, no socket, so it runs locally. Mutation-checked: bypassing the config gate
makes the rejection test fail.
Documents the pattern that makes a passthrough proxy mode unnecessary. If what you want is vaulted API keys, an audit trail, quotas and cost tracking -- not agent logic -- a minimal LLM-only agent behind this adapter already provides all of it and behaves like a plain model call. Building an actual proxy would mean entering the LLM-gateway market against LiteLLM, Portkey and Cloudflare AI Gateway with a worse product, inheriting per-provider streaming/tool/vision passthrough maintenance for zero agent value, and shipping a feature with neither logic nor configuration -- the opposite of Pillar 1. The limits are stated before the recipe, not after: single-turn only (the adapter sends just the last user message), one agent per model (a caller cannot route to an arbitrary 'gpt-4o'), and no caching, fallbacks, load balancing or virtual per-user keys. If you need those, use a real gateway. Every config shape is taken from working examples in this repo -- agent and workflow from docs/agent-configs/agent-father, the LLM task from docs/langchain.md, the vault reference syntax from docs/a2a-protocol.md -- rather than invented. The one deliberate departure from AGENTS.md 5.3 (a rule with no actionmatcher on lastStep) is called out with its reason: firing on every turn is the intent for a gateway, not an ordering bug. The recipe is labelled as an unverified pattern, since the adapter's test suite does not exercise it end to end.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds a configurable ChangesOpenAI-compatible API adapter
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant OpenAiAuthFilter
participant RestOpenAiAdapter
participant AgentModelResolver
participant OpenAiConversationBridge
participant OpenAiSseWriter
Client->>OpenAiAuthFilter: Send authenticated /v1 chat request
OpenAiAuthFilter->>RestOpenAiAdapter: Provide resolved user identity
RestOpenAiAdapter->>AgentModelResolver: Resolve requested model
RestOpenAiAdapter->>OpenAiConversationBridge: Prepare and execute conversation turn
OpenAiConversationBridge->>OpenAiSseWriter: Emit streaming chunks and usage
OpenAiSseWriter-->>Client: Return SSE frames and [DONE]
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| this.authorizationEnabled = authorizationEnabled; | ||
| } | ||
|
|
||
| void onStart(@Observes StartupEvent event) { |
There was a problem hiding this comment.
Pull request overview
Adds a new /v1 OpenAI-compatible adapter layer so Open WebUI and OpenAI-protocol SDK clients can drive EDDI conversations without knowing EDDI’s native API, while keeping all downstream processing inside IConversationService (rules, tools, RAG, HITL, memory, etc.). The adapter is disabled by default and is wired via additive configuration and a new integration package.
Changes:
- Introduces
ai.labs.eddi.integrations.openaiwith endpoints for/v1/modelsand/v1/chat/completions(JSON + SSE), plus auth, startup guard, model resolution, message mapping, and SSE framing. - Adds configuration and Quarkus HTTP auth-permission wiring for
/v1/*to support either shared API key mode or OIDC mode. - Adds extensive unit test coverage for binding, wire format, SSE framing, auth behavior, model resolution, message/attachment mapping, and conversation-bridging behavior; adds end-user documentation and changelog entry.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java | New /v1 JAX-RS resource for models + chat completions (sync + SSE), concurrency bounding |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridge.java | Bridges stateless OpenAI requests to stateful EDDI conversations (mapping, HITL handling, streaming) |
| src/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.java | Lists/resolves deployed agents as OpenAI “models”, including stateless variants and ambiguity handling |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapper.java | Maps OpenAI messages[] (incl. multimodal parts) into EDDI InputData + attachment_N contexts |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java | Writes OpenAI-compatible SSE chunk framing to an output stream |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java | Auth + identity resolution filter for /v1/* (api-key mode or OIDC mode) |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiStartupGuard.java | Fails startup on insecure “enabled + unauthenticated” combinations when auth is expected |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiCompatConfig.java | Config carrier for eddi.openai-compat.* |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiApiException.java | Exception type carrying OpenAI-style error envelope metadata |
| src/main/java/ai/labs/eddi/integrations/openai/OpenAiExceptionMapper.java | ExceptionMapper rendering OpenAiApiException as OpenAI error envelope |
| src/main/java/ai/labs/eddi/integrations/openai/model/OpenAiErrorResponse.java | DTO for OpenAI error envelope |
| src/main/java/ai/labs/eddi/integrations/openai/model/ModelsResponse.java | DTO for GET /v1/models response |
| src/main/java/ai/labs/eddi/integrations/openai/model/ModelObject.java | DTO for model entries (snake_case fields) |
| src/main/java/ai/labs/eddi/integrations/openai/model/ContentPart.java | DTO for polymorphic multimodal content parts (text/image/file/audio) |
| src/main/java/ai/labs/eddi/integrations/openai/model/ChunkChoice.java | DTO for streaming chunk choice / delta semantics |
| src/main/java/ai/labs/eddi/integrations/openai/model/Choice.java | DTO for non-streaming chat completion choices |
| src/main/java/ai/labs/eddi/integrations/openai/model/ChatMessage.java | DTO for OpenAI chat messages with polymorphic content |
| src/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionResponse.java | DTO for non-streaming chat completion response |
| src/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionRequest.java | DTO for chat completion request (tolerant binding, user as JsonNode, stateless extension) |
| src/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionChunk.java | DTO for streaming completion chunks |
| src/main/resources/application.properties | Adds eddi.openai-compat.* settings and explicit /v1/* auth permission entry |
| src/test/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapterTest.java | Tests request-shaping logic (not QuarkusTest) incl. stateless override + config gate |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiWireFormatTest.java | Pins exact JSON serialization shape (snake_case fields, omission behavior) |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiTestFixtures.java | Shared test fixtures / config builder |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiStartupGuardTest.java | Tests startup guard failure/pass combinations |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiSseWriterTest.java | Tests byte-level SSE framing, flushing, idempotence, disconnect tolerance |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapperTest.java | Tests mapping of text/system/multimodal/file/audio parts into EDDI input + contexts |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridgeTest.java | Tests conversation isolation, mapping reuse/races, HITL discrimination, sync + streaming behavior |
| src/test/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilterTest.java | Tests shared-key auth + identity resolution + OIDC mode behavior |
| src/test/java/ai/labs/eddi/integrations/openai/AgentModelResolverTest.java | Tests model id building, ambiguity guards, stateless suffix behavior, caching |
| docs/open-webui-integration.md | End-user integration guide for Open WebUI + OpenAI-protocol clients |
| docs/changelog.md | Changelog entry documenting the new adapter and design decisions |
| AGENTS.md | Adds roadmap row referencing the new OpenAI-compatible API adapter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
src/test/java/ai/labs/eddi/integrations/openai/OpenAiTestFixtures.java (1)
31-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse top-level imports for
ConsumerandOptional.The Java guidelines require simple names with top-level imports; avoid fully qualified types inline.
Proposed fix
package ai.labs.eddi.integrations.openai; +import java.util.Optional; +import java.util.function.Consumer; + import ai.labs.eddi.engine.model.Deployment.Environment; ... -static OpenAiCompatConfig config(java.util.function.Consumer<ConfigBuilder> customizer) { +static OpenAiCompatConfig config(Consumer<ConfigBuilder> customizer) { ... - return new OpenAiCompatConfig(enabled, java.util.Optional.ofNullable(apiKey), httpPolicy, + return new OpenAiCompatConfig(enabled, Optional.ofNullable(apiKey), httpPolicy,As per coding guidelines, Java files must use simple names with top-level imports.
Also applies to: 51-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiTestFixtures.java` around lines 31 - 34, Update OpenAiTestFixtures to import Consumer and Optional at the top of the file, then replace their fully qualified inline usages in config and the additional referenced code with the simple type names. Preserve the existing behavior and method signatures.Source: Coding guidelines
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java (1)
147-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo metric for concurrency-limit rejections.
inFlight.tryAcquire()failures (both the sync path at Lines 147-149 and the streaming body at Lines 192-196) surface only as a 429/in-band notice; there's no Micrometer counter tracking how often the adapter is saturated, unlike the turn-level metrics already present inOpenAiConversationBridge. A simple counter here would help diagnose whethereddi.openai-compat.max-concurrent-requestsneeds tuning in production.Based on coding guidelines: "Add Micrometer counters, timers, or gauges to new features and initialize them with
@PostConstruct."Also applies to: 192-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java` around lines 147 - 149, In RestOpenAiAdapter, add a Micrometer counter initialized via `@PostConstruct` and increment it whenever inFlight.tryAcquire() rejects a request, covering both the synchronous path and streaming body path. Use the counter to track concurrency-limit rejections while preserving the existing busy exception and in-band notice behavior.Source: Coding guidelines
src/main/java/ai/labs/eddi/integrations/openai/OpenAiExceptionMapper.java (1)
24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog 5xx renderings before returning them.
Server-side failures are converted to a client envelope with no server-side trace, so a 500 from
/v1leaves nothing in the logs to diagnose. As per coding guidelines, "Use JBoss Logger rather thanSystem.out, include conversation context, and select appropriate DEBUG, INFO, and ERROR levels."♻️ Proposed refactor
+ private static final Logger LOGGER = Logger.getLogger(OpenAiExceptionMapper.class); + `@Override` public Response toResponse(OpenAiApiException exception) { + if (exception.getStatus() >= 500) { + LOGGER.errorf(exception, "OpenAI-compatible request failed (status=%d, code=%s): %s", + exception.getStatus(), exception.getCode(), exception.getMessage()); + } else { + LOGGER.debugf("OpenAI-compatible request rejected (status=%d, code=%s)", + exception.getStatus(), exception.getCode()); + } return Response.status(exception.getStatus())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiExceptionMapper.java` around lines 24 - 30, Update OpenAiExceptionMapper.toResponse to log server-side (5xx) OpenAiApiException responses before building and returning the client error envelope. Use the project’s JBoss Logger, include available conversation context and exception details, and keep lower-status responses on their existing path without adding unnecessary logging.Source: Coding guidelines
src/test/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilterTest.java (1)
59-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a static import instead of the inlined fully qualified
org.mockito.Mockito.doAnswer.As per coding guidelines, "Use simple names with top-level imports; do not inline fully qualified names except when disambiguating same-named types."
♻️ Proposed refactor
+import static org.mockito.Mockito.doAnswer; @@ - org.mockito.Mockito.doAnswer(inv -> { + doAnswer(inv -> { properties.put(inv.getArgument(0), inv.getArgument(1)); return null; }).when(requestContext).setProperty(any(), any());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilterTest.java` around lines 59 - 62, Replace the fully qualified org.mockito.Mockito.doAnswer invocation in the requestContext mock setup with a static import for doAnswer, while preserving the existing lambda behavior and when(requestContext).setProperty(any(), any()) configuration.Source: Coding guidelines
src/main/java/ai/labs/eddi/integrations/openai/OpenAiApiException.java (1)
26-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a cause-carrying constructor; use
Response.Status.TOO_MANY_REQUESTSinstead of the literal.
serverError/unavailable/timeoutare the wrappers for downstream failures, but there is no way to attach the original exception, so the root cause is lost from logs.♻️ Proposed refactor
public OpenAiApiException(int status, String type, String code, String message) { - super(message); + this(status, type, code, message, null); + } + + public OpenAiApiException(int status, String type, String code, String message, Throwable cause) { + super(message, cause); this.status = status; this.type = type; this.code = code; } @@ public static OpenAiApiException busy(String message) { - return new OpenAiApiException(429, OpenAiErrorResponse.TYPE_RATE_LIMIT, null, message); + return new OpenAiApiException(Response.Status.TOO_MANY_REQUESTS.getStatusCode(), + OpenAiErrorResponse.TYPE_RATE_LIMIT, null, message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiApiException.java` around lines 26 - 55, Update OpenAiApiException with a cause-carrying constructor that delegates to the existing message/status/type/code initialization and preserves the original throwable via the superclass. Use this constructor in the serverError, unavailable, and timeout factory methods so downstream failures retain their causes, and replace the literal 429 in busy with Response.Status.TOO_MANY_REQUESTS.getStatusCode().src/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.java (1)
91-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider registering Micrometer instruments for catalogue rebuilds and unknown/ambiguous model resolutions.
As per coding guidelines, "Add Micrometer counters, timers, or gauges to new features and initialize them with
@PostConstruct." A rebuild timer plus counters for unknown/ambiguous resolutions would make model-routing failures visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.java` around lines 91 - 97, Update AgentModelResolver.initCache and the surrounding model-resolution flow to initialize Micrometer instruments in a `@PostConstruct` method: add a timer for catalogue rebuilds and counters for unknown and ambiguous model resolutions. Record the timer around rebuild work and increment the corresponding counter whenever each resolution outcome occurs, using the project’s established MeterRegistry integration.Source: Coding guidelines
src/test/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridgeTest.java (1)
429-485: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace inlined fully qualified names with imports.
java.io.ByteArrayOutputStreamandjava.nio.charset.StandardCharsetsare spelled out at Lines 434, 438, 450, 452, 454, 465, 467, 469, 480, 482 and 484; the same applies tojava.util.function.Consumer(Line 119) andai.labs.eddi.engine.model.Context(Line 519). None disambiguate a same-named type.As per coding guidelines: "Use simple names with top-level imports; do not inline fully qualified names except when disambiguating same-named types, and remove unused imports."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridgeTest.java` around lines 429 - 485, Replace the fully qualified references to ByteArrayOutputStream, StandardCharsets, Consumer, and Context in OpenAiConversationBridgeTest with simple names backed by top-level imports. Remove any imports made unused by this change, while preserving the existing test behavior.Source: Coding guidelines
src/test/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapperTest.java (1)
324-343: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for
file_datathat is not adata:URI.Every file-part test sends a well-formed
data:URI. A bare base64file_data(e.g."abc,def") currently reachesfromDataUriunchecked and throws — see the comment onOpenAiMessageMapper.javaLines 264-295. A test asserting the part is skipped or accepted, rather than blowing up the turn, would pin that behaviour.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapperTest.java` around lines 324 - 343, The file-part tests lack coverage for bare, non-data-URI file_data values. Add a test alongside filePart_withoutDataOrId_isSkipped and filePart_unnamed_stillGetsAnAttachment using a value such as “abc,def”, and assert the mapper handles it without throwing by verifying the intended skipped or accepted attachment behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/open-webui-integration.md`:
- Around line 418-436: The behavior example in `docs/open-webui-integration.md`
must comply with the repository’s actionmatcher requirement: update the `Always
answer` rule’s conditions to include an appropriate `actionmatcher` targeting
`lastStep`, while preserving its intended per-turn behavior, and remove the
statement claiming an intentional departure from `AGENTS.md`.
- Around line 135-150: Update the fenced Markdown blocks around the “A turn, end
to end” section and the block at the later referenced section: add appropriate
language identifiers to each fence to satisfy MD040, and remove the blank line
inside the blockquote so it remains a continuous block and satisfies MD028.
In `@src/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.java`:
- Around line 288-296: Update the model catalogue population around
byModelId.putIfAbsent in AgentModelResolver to detect when an existing entry
already uses the same lowercased modelId for a different agent. Log a clear
collision warning that includes the modelId and affected agent identifiers,
while preserving the existing first-entry selection and other index population
behavior.
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiCompatConfig.java`:
- Around line 49-73: Validate constructor inputs in OpenAiCompatConfig: require
httpPolicy to be exactly an accepted policy value, including rejecting typos,
case differences, and surrounding whitespace, and reject non-positive
requestTimeoutSeconds and maxConcurrentRequests. Fail fast during construction
with a clear validation exception before assigning these fields, while
preserving valid configuration behavior.
In
`@src/main/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridge.java`:
- Around line 237-241: Update the exception handling in OpenAiConversationBridge
methods around the conversation-start and related failure paths (including the
code at lines 223, 240, and 476) so OpenAiApiException.serverError receives a
generic client-facing message instead of concatenating e.getMessage(). Keep the
detailed exception message in LOGGER.errorf for internal diagnostics.
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapper.java`:
- Around line 264-295: Update fromDataUri to validate the data: scheme before
slicing metadata. When the input lacks the prefix, treat it as raw base64 by
creating the attachment with the filename-derived fallback MIME and storing the
entire input as attachment data; retain the existing malformed-separator
handling and data-URI parsing for prefixed inputs.
- Around line 253-262: Update the logging in OpenAiMessageMapper to sanitize all
client-supplied values before interpolation, including fileName, file.fileId(),
and audio.format() near the referenced audio logging. Reuse
ai.labs.eddi.utils.LogSanitizer.sanitize consistently while preserving the
existing warning messages and control flow.
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java`:
- Around line 103-121: Update emit in OpenAiSseWriter so Jackson serialization
exceptions such as WriteValueException or JsonProcessingException are caught
before the general IOException handler. Route serialization failures to the
existing warning-and-drop behavior without calling markBroken, while retaining
markBroken for genuine IO write or flush failures.
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiStartupGuard.java`:
- Around line 73-76: Update OpenAiStartupGuard.isUnprotected so keyless permit
mode is allowed only when eddi.security.allow-unauthenticated is explicitly
enabled, independent of authorization.enabled or global OIDC state; retain the
existing OIDC and API-key protections. In
src/main/resources/application.properties lines 188-203, document that permit
mode requires an API key unless this escape hatch is enabled. In
planning/openai-api-adapter-plan.md lines 462-471, update the startup-guard
predicate and security model to match.
In `@src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java`:
- Around line 96-115: Update chatCompletions and streamingResponse to populate
every response or chunk model field with the resolved model’s
canonicalModelId(), matching retrieveModel and GET /v1/models. Stop using the
raw request.model() value after model resolution, while preserving the existing
response and streaming behavior.
---
Nitpick comments:
In `@src/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.java`:
- Around line 91-97: Update AgentModelResolver.initCache and the surrounding
model-resolution flow to initialize Micrometer instruments in a `@PostConstruct`
method: add a timer for catalogue rebuilds and counters for unknown and
ambiguous model resolutions. Record the timer around rebuild work and increment
the corresponding counter whenever each resolution outcome occurs, using the
project’s established MeterRegistry integration.
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiApiException.java`:
- Around line 26-55: Update OpenAiApiException with a cause-carrying constructor
that delegates to the existing message/status/type/code initialization and
preserves the original throwable via the superclass. Use this constructor in the
serverError, unavailable, and timeout factory methods so downstream failures
retain their causes, and replace the literal 429 in busy with
Response.Status.TOO_MANY_REQUESTS.getStatusCode().
In `@src/main/java/ai/labs/eddi/integrations/openai/OpenAiExceptionMapper.java`:
- Around line 24-30: Update OpenAiExceptionMapper.toResponse to log server-side
(5xx) OpenAiApiException responses before building and returning the client
error envelope. Use the project’s JBoss Logger, include available conversation
context and exception details, and keep lower-status responses on their existing
path without adding unnecessary logging.
In `@src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java`:
- Around line 147-149: In RestOpenAiAdapter, add a Micrometer counter
initialized via `@PostConstruct` and increment it whenever inFlight.tryAcquire()
rejects a request, covering both the synchronous path and streaming body path.
Use the counter to track concurrency-limit rejections while preserving the
existing busy exception and in-band notice behavior.
In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilterTest.java`:
- Around line 59-62: Replace the fully qualified org.mockito.Mockito.doAnswer
invocation in the requestContext mock setup with a static import for doAnswer,
while preserving the existing lambda behavior and
when(requestContext).setProperty(any(), any()) configuration.
In
`@src/test/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridgeTest.java`:
- Around line 429-485: Replace the fully qualified references to
ByteArrayOutputStream, StandardCharsets, Consumer, and Context in
OpenAiConversationBridgeTest with simple names backed by top-level imports.
Remove any imports made unused by this change, while preserving the existing
test behavior.
In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapperTest.java`:
- Around line 324-343: The file-part tests lack coverage for bare, non-data-URI
file_data values. Add a test alongside filePart_withoutDataOrId_isSkipped and
filePart_unnamed_stillGetsAnAttachment using a value such as “abc,def”, and
assert the mapper handles it without throwing by verifying the intended skipped
or accepted attachment behavior.
In `@src/test/java/ai/labs/eddi/integrations/openai/OpenAiTestFixtures.java`:
- Around line 31-34: Update OpenAiTestFixtures to import Consumer and Optional
at the top of the file, then replace their fully qualified inline usages in
config and the additional referenced code with the simple type names. Preserve
the existing behavior and method signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 745ca7c4-b6dc-4213-bde3-3b67670a3d78
📒 Files selected for processing (34)
AGENTS.mddocs/changelog.mddocs/open-webui-integration.mdplanning/openai-api-adapter-plan.mdsrc/main/java/ai/labs/eddi/integrations/openai/AgentModelResolver.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiApiException.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiCompatConfig.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridge.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiExceptionMapper.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapper.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.javasrc/main/java/ai/labs/eddi/integrations/openai/OpenAiStartupGuard.javasrc/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionChunk.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionRequest.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ChatCompletionResponse.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ChatMessage.javasrc/main/java/ai/labs/eddi/integrations/openai/model/Choice.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ChunkChoice.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ContentPart.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ModelObject.javasrc/main/java/ai/labs/eddi/integrations/openai/model/ModelsResponse.javasrc/main/java/ai/labs/eddi/integrations/openai/model/OpenAiErrorResponse.javasrc/main/resources/application.propertiessrc/test/java/ai/labs/eddi/integrations/openai/AgentModelResolverTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilterTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridgeTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiMessageMapperTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiSseWriterTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiStartupGuardTest.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiTestFixtures.javasrc/test/java/ai/labs/eddi/integrations/openai/OpenAiWireFormatTest.javasrc/test/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapterTest.java
Closes the gap the unit tests structurally could not reach. The adapter serves sync and streaming from a SINGLE JAX-RS method, dispatching on the 'stream' body field rather than the Accept header, because openai-python hardcodes Accept: application/json regardless of stream=True and Open WebUI sends no Accept header at all. A refactor toward content negotiation -- two methods with different @produces -- would look correct in every unit test and silently return JSON to every streaming client. That guard can only be written at the HTTP layer, which is why this test exists: * stream:true + Accept: application/json -> text/event-stream * stream:true + no Accept header -> text/event-stream * stream:false + Accept: text/event-stream -> application/json 16 tests in total, also covering the response wire shape (including that finish_reason and owned_by are snake_cased and usage is absent), per-chat conversation isolation end to end, both stateless routes, the error envelopes, and tolerance of the unknown fields every client sends. Deploys the shared minimal agent (parser/rules/output/templating) rather than mocking, so the whole path is exercised. No LLM, so no provider credentials are needed and responses are deterministic. Models are addressed by bare agent id, a documented resolution route, so the test does not depend on how the shared fixture names its descriptor. CI ONLY. Quarkus cannot boot in the dev sandbox -- 'Unable to establish loopback connection' -- the same environmental limit that already fails SlackWebApiClientTest. The test compiles and failsafe discovers all 16 cases locally, but whether they pass is for CI to determine; I have not seen them go green.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:146
requestcan be null when the POST body is missing/empty, butrequest.isStreaming()is dereferenced unconditionally. This will throw a NullPointerException and return a 500 instead of an OpenAI-shaped 400 error.
if (request.isStreaming()) {
return streamingResponse(turn, completionId, request.model(), created);
src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java:92
finish()writes the[DONE]frame but does not flush afterwards. On some servlet/proxy stacks this can delay delivery of the terminator, making clients think the stream is still open.
finished = true;
role();
emit(ChunkChoice.finish(finishReason));
write(DONE_FRAME);
}
src/main/java/ai/labs/eddi/integrations/openai/OpenAiConversationBridge.java:476
- When an exception has a null message (e.g., NPE), the non-streaming error path logs and returns "...: null". Using the existing
describe()helper avoids leaking the literal word "null" to callers and keeps messages consistent with the streaming path.
LOGGER.errorf("OpenAI adapter turn failed: %s", e.getMessage());
return OpenAiApiException.serverError(null, "The agent could not process the message: " + e.getMessage());
CI's Secret Scanning job failed on five findings, all in documentation and all false positives: four curl-auth-header hits on literal 'Authorization: Bearer sk-...' strings, and one generic-api-key anchored on the 'api-key' keyword inside an ASCII request diagram. No finding was in source code -- notably the integration test's API key constant was not flagged. Fixed at the source rather than suppressed via .gitleaksignore. Fingerprints there are commit-scoped, so a suppression would need re-adding every time the line moves, and it would hide the pattern rather than remove it. Parameterising the credential is also better documentation practice: readers should not be shown a curl command that puts a bearer token into their shell history. curl -H "Authorization: Bearer $EDDI_API_KEY" ... client = OpenAI(..., api_key=os.environ["EDDI_API_KEY"]) The diagram placeholder '<api-key>' becomes '<token>', which reads the same without the keyword the generic rule anchors on. Everything else in this CI run was green, including the new OpenAiCompatIT: 16 tests, 0 failures -- so the stream-dispatch design is now verified against a real HTTP stack rather than only reasoned about.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:146
chatCompletionsdereferencesrequest(request.isStreaming(),request.model()) even though an empty/missing JSON body can be bound asnull, which will throw a NullPointerException and bypass the OpenAI error envelope. This also makes the "missing model" case surface as a 404 instead of a 400. Validate the request (and its requiredmodel) up front and fail with a 400 OpenAI error before resolving the model.
requireEnabled();
var model = applyStatelessOverride(resolveOrFail(request == null ? null : request.model()), request);
String userId = requireUserId(requestContext);
Map<String, String> headers = flatten(httpHeaders);
if (!inFlight.tryAcquire()) {
throw OpenAiApiException.busy("Too many concurrent requests. Please retry shortly.");
}
try {
var turn = bridge.prepare(model, request, headers, userId);
String completionId = "chatcmpl-eddi-" + UUID.randomUUID();
long created = Instant.now().getEpochSecond();
if (request.isStreaming()) {
return streamingResponse(turn, completionId, request.model(), created);
src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java:92
OpenAiSseWriter.finish()writes the[DONE]sentinel but never flushes afterward. If the underlying output stream buffers (or the container doesn’t flush on close promptly), clients may not observe the terminator in a timely way, which can look like a hung stream.
public void finish(String finishReason) {
if (finished) {
return;
}
finished = true;
role();
emit(ChunkChoice.finish(finishReason));
write(DONE_FRAME);
}
…ndings The previous commit parameterised the curl and Python examples, but Secret Scanning still failed with the same five findings at the same line numbers -- because gitleaks scans the PR's commit range, not the working tree. The literals remain in 3b384d9 and 5283853, and no later commit can remove them without rewriting history, which this repo forbids. .gitleaksignore is the intended mechanism for exactly this, and its fingerprints are commit-scoped precisely because the scan walks history. Five entries added with the justification the file's header asks for. Both halves are wanted: these entries clear the historical findings, and the previous commit's env-var examples stop any future commit re-triggering the rule. All five are placeholder credentials in documentation; none was ever a real secret, and none was in source code.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 45 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:145
chatCompletions()tolerates a nullrequestwhen resolving the model, but then unconditionally dereferencesrequest(request.isStreaming(),request.model(),request.includeUsage()), which will throw a NullPointerException if the JSON body is missing/empty (RESTEasy Reactive can pass null for an empty body). This should fail with an OpenAI-shaped 400 instead of a 500.
requireEnabled();
var model = applyStatelessOverride(resolveOrFail(request == null ? null : request.model()), request);
String userId = requireUserId(requestContext);
Map<String, String> headers = flatten(httpHeaders);
…eDoS Two of the three CodeQL alerts on this PR. log-injection (medium, PostgresUserConversationStore): the delete path logged `intent` unsanitized. The OpenAI adapter builds that intent from a request header, so a newline in a chat id could forge log entries. Routed through the existing LogSanitizer, as the bridge already does. The Mongo store has no logging, so nothing to mirror there. polynomial-redos (high, AgentModelResolver.slugify): replaces the dash trim `(^-+|-+$)` with a character walk. This is NOT a fixed vulnerability. The alert is a false positive twice over — the preceding NON_SLUG_CHARS pass collapses runs so `-+` can never match more than one character, and the regex measures linear regardless (3ms on 400k separators; the engine anchors on `$` instead of backtracking). It is replaced because a standing high-severity alert competes for attention with real ones, and the replacement is no harder to read. The first version of this change carried a timing assertion. Measuring the old regex showed it would have passed against both implementations, so it was removed rather than shipped: a test that cannot fail is worse than none. What remains pins trimming behaviour across the swap, and is mutation-checked — stubbing out the trim kills 3 tests. The third alert (java/user-controlled-bypass in OpenAiAuthFilter) is untouched pending review; it is auth code and the early return on a non-/v1 path looks inherent to any path-scoped filter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:171
- The Javadoc says
constantTimeEqualsdoes not leak the length relationship, butMessageDigest.isEqualreturns immediately when the byte array lengths differ. Either adjust the comment to avoid over-claiming, or implement a length-hiding comparison if that property is required.
/**
* Compare secrets without leaking their length relationship through timing.
* {@code String.equals} short-circuits on the first differing character.
*/
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:145
chatCompletions(...)tolerates a nullrequestinapplyStatelessOverride(...), but then unconditionally callsrequest.isStreaming()/request.model()/request.includeUsage(), which will throw a NullPointerException on an empty or unreadable body. This should fail as a 400 OpenAI error envelope instead.
requireEnabled();
var model = applyStatelessOverride(resolveOrFail(request == null ? null : request.model()), request);
String userId = requireUserId(requestContext);
Map<String, String> headers = flatten(httpHeaders);
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:94
- When user identity cannot be resolved, the filter aborts via
abort(...), which always builds anOpenAiApiException.unauthorized(...)envelope. That forceserror.code="invalid_api_key"even when the API key was correct and the failure is "missing/invalid user identity". This can mislead clients/operators; consider omittingcode(null) for this case or introducing a distinct adapter code.
This issue also appears on line 168 of the same file.
abort(requestContext, Response.Status.UNAUTHORIZED.getStatusCode(),
"Could not determine the calling user. Send X-OpenWebUI-User-Id "
+ "(with eddi.openai-compat.trust-user-headers=true), authenticate via OIDC, "
+ "or set eddi.openai-compat.allow-anonymous=true.");
return;
src/main/java/ai/labs/eddi/datastore/postgres/PostgresUserConversationStore.java:144
- PR description says "no existing Java file is modified", but this PR updates
PostgresUserConversationStore. Please update the PR description to match the actual change set (or clarify that there is one non-adapter Java change for log sanitization).
// Sanitized: the intent embeds a caller-supplied chat key (the OpenAI
// adapter builds channel:openai:<agentId>:<chatKey> from a request
// header), so a newline in it could forge log entries.
LOGGER.error("Failed to delete user conversation intent=" + sanitize(intent), e);
…th a test CodeQL reports java/user-controlled-bypass against the early return in OpenAiAuthFilter.filter: a user-controlled path decides whether the API key check runs. It does, but the filter is neither the only nor the first check in front of those paths. application.properties ends with quarkus.http.auth.permission.authenticated.paths=/,/* at policy authenticated, and Quarkus HTTP authorization runs before JAX-RS filters, so every path the filter declines has already been required to authenticate. The sole exception is /v1/*, exempted to 'permit' precisely so the shared API key can be checked in the filter instead of being rejected at the OIDC layer as a malformed JWT — and that is exactly the set isGuarded returns true for. The guard chooses between the adapter's key check and Quarkus' own, not between authenticated and anonymous. Records that on isGuarded rather than only in the GitHub dismissal, so it is found where the suspicious-looking code is. Adds a test, because the argument rests entirely on two config lines and a finding dismissed on unchecked config is how a real bypass ships. It asserts the catch-all paths, the catch-all policy and the /v1/* exemption against src/main/resources/application.properties; flipping the policy to permit fails it. It reads from source deliberately: the first version loaded the file from the classpath, where src/test/resources shadows it and declares none of these keys.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:146
chatCompletionspartially treats the request body as nullable (it null-checks before readingmodel), but then immediately dereferencesrequest(request.isStreaming(),request.model(),request.includeUsage()), which will throw an NPE if the body is empty/unparseable and JAX-RS suppliesnull. This should fail with an OpenAI-shaped 400 instead of a 500.
requireEnabled();
var model = applyStatelessOverride(resolveOrFail(request == null ? null : request.model()), request);
String userId = requireUserId(requestContext);
Map<String, String> headers = flatten(httpHeaders);
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:195
- The Javadoc claims
constantTimeEqualsavoids leaking the length relationship, butMessageDigest.isEqual()returns immediately when array lengths differ. Hashing first (fixed-length digests) avoids that short-circuit and better matches the comment’s intent.
/**
* Compare secrets without leaking their length relationship through timing.
* {@code String.equals} short-circuits on the first differing character.
*/
private static boolean constantTimeEquals(String presented, String expected) {
return MessageDigest.isEqual(presented.getBytes(StandardCharsets.UTF_8),
expected.getBytes(StandardCharsets.UTF_8));
}
src/test/java/ai/labs/eddi/integration/OpenAiCompatIT.java:40
- This PR adds HTTP-level coverage for the
/v1adapter (this@QuarkusTestIT), but the PR description’s “Known gap” section says no real-HTTP test exists. Please update the PR description (or adjust this test’s scope) so reviewers don’t miss that the gap is already addressed here.
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:96 - When user identity can’t be resolved, the filter aborts with
OpenAiApiException.unauthorized(...), which always useserror.code=invalid_api_key(seeOpenAiApiException.unauthorized). That code is accurate for a missing/wrong API key, but misleading for “missing X-OpenWebUI-User-Id / anonymous identity refused” and may cause clients to prompt users to change the API key instead of fixing identity forwarding.
String userId = resolveUserId(requestContext);
if (userId == null) {
abort(requestContext, Response.Status.UNAUTHORIZED.getStatusCode(),
"Could not determine the calling user. Send X-OpenWebUI-User-Id "
+ "(with eddi.openai-compat.trust-user-headers=true), authenticate via OIDC, "
+ "or set eddi.openai-compat.allow-anonymous=true.");
return;
}
requestContext.setProperty(PROP_USER_ID, userId);
src/main/java/ai/labs/eddi/datastore/postgres/PostgresUserConversationStore.java:145
- PR description says “no existing Java file is modified”, but this change updates an existing core datastore class. Please update the PR description to reflect this (even if the change is intentional and small) so reviewers don’t miss non-adapter edits.
// Sanitized: the intent embeds a caller-supplied chat key (the OpenAI
// adapter builds channel:openai:<agentId>:<chatKey> from a request
// header), so a newline in it could forge log entries.
LOGGER.error("Failed to delete user conversation intent=" + sanitize(intent), e);
}
# Conflicts: # docs/changelog.md
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java:113
finish()writes the finaldata: [DONE]frame but never flushes the stream afterward. This contradicts the class contract (“Every frame is flushed immediately”) and can leave some clients/proxies waiting on the sentinel until the connection closes.
public void finish(String finishReason) {
if (finished) {
return;
}
finished = true;
role();
emit(ChunkChoice.finish(finishReason));
if (includeUsage && usage != null) {
emitChunk(ChatCompletionChunk.usageOnly(id, model, created, usage));
}
write(DONE_FRAME);
}
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:96
- When user identity cannot be resolved (missing/blank X-OpenWebUI-User-Id in api-key mode, or anonymous OIDC identity), the filter aborts with a 401 but uses
OpenAiApiException.unauthorized(...), which hardcodes the OpenAI errorcodetoinvalid_api_key. This makes identity/authorization failures indistinguishable from a bad API key for clients and operators.
String userId = resolveUserId(requestContext);
if (userId == null) {
abort(requestContext, Response.Status.UNAUTHORIZED.getStatusCode(),
"Could not determine the calling user. Send X-OpenWebUI-User-Id "
+ "(with eddi.openai-compat.trust-user-headers=true), authenticate via OIDC, "
+ "or set eddi.openai-compat.allow-anonymous=true.");
return;
}
requestContext.setProperty(PROP_USER_ID, userId);
The merge left the 2026-07-28 "code-review findings wave 1" entry below a 2026-07-27 one, in a file that is explicitly newest-first. Moves that one block above the 07-27 OpenAI adapter entry; no other entry changes position and no content changes. Also repairs two seam defects my own merge resolution introduced: a doubled "---" before the wave-1 entry, and a missing one before the Keycloak entry. Deliberately a targeted block move, not a re-sort. Sorting the file by date was tried first and reverted: `---` is not a 1:1 entry delimiter here (355 headings, 339 separators), so splitting on it does not yield entries, and the sort rewrote ~37% of an 11.6k-line historical document and pushed undated entries to the bottom. This version is anchored on "## " headings and asserts that the set of non-blank, non-separator lines is unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/main/java/ai/labs/eddi/integrations/openai/model/ContentPart.java:53
isInputAudio()treats an empty/blank base64 payload as valid, which can produce an attachment with blankdataand trigger warnings/drops downstream (AttachmentContextExtractor requires non-blankurl/data). Consider requiring non-blankdatahere.
public boolean isInputAudio() {
return TYPE_INPUT_AUDIO.equals(type) && inputAudio != null && inputAudio.data() != null;
}
src/main/java/ai/labs/eddi/integrations/openai/model/ContentPart.java:49
isImageUrl()treats an empty/blank URL as valid, which can produce an attachment with a blankurland trigger warnings/drops downstream (AttachmentContextExtractor requires non-blankurl/data). It’s better to treat blank URLs as absent at the binding layer.
public boolean isImageUrl() {
return TYPE_IMAGE_URL.equals(type) && imageUrl != null && imageUrl.url() != null;
}
src/main/java/ai/labs/eddi/integrations/openai/OpenAiSseWriter.java:94
usage()’s Javadoc says usage is ignored when the caller didn’t request it, but the method currently stores usage unconditionally. This is harmless but inconsistent and can be made true to the comment by short-circuiting whenincludeUsageis false (and when the usage itself is null).
public void usage(TokenUsage tokenUsage) {
this.usage = tokenUsage;
}
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:157
requestcan be null (e.g., missing/empty JSON body), but the method later callsrequest.isStreaming(),request.model()andrequest.includeUsage()unconditionally, which will throw a NullPointerException and return a 500 instead of an OpenAI-shaped 400.
requireEnabled();
var model = applyStatelessOverride(resolveOrFail(request == null ? null : request.model()), request);
String userId = requireUserId(requestContext);
Map<String, String> headers = flatten(httpHeaders);
# Conflicts: # docs/changelog.md
Two defects, both found by re-running the stack rather than reading it. Adding an LLM key later silently did nothing. The seeder guarded on "is any model exposed" and exit 0'd if so. The MongoDB volume persists, so the most common second run is exactly that case: the rule-based agent is already there and the user has now set EDDI_DEMO_LLM_API_KEY to get an agent that can actually answer questions. They got no new model and no explanation. The guard is now per agent, keyed on the model-id prefix each descriptor name slugifies to. Vault storage moved into its own step that runs whenever a key is supplied, so changing the key rotates it instead of being skipped behind an "already exists" check. The closing "Ready" listing could omit the agent just created. The poll exited on grep -q '"id"', which a pre-existing agent satisfies instantly, so a freshly deployed LLM agent still inside the adapter's 30s model-cache TTL was absent from the output and looked like a failure. Confirmed it was display-only by re-querying after the TTL. The loop now waits for the prefixes actually expected on this run. Verified live against a populated volume: run 1 created the rule-based agent, run 2 reported "already deployed - skipping" and created no duplicate, run 3 with a key added the LLM agent and listed all four models. The old code stopped at run 2. Also: the final "open this URL" line hardcoded port 3000 and was wrong under OPEN_WEBUI_PORT, so compose passes OPEN_WEBUI_URL. .env.example gained an Open WebUI section, since the demo's .env is gitignored and these variables were undiscoverable from a fresh clone. Docs gained subsections for re-running, port collisions and down vs down -v - all three bit during testing. Doc accuracy pass found no drift: all 11 eddi.openai-compat.* keys and defaults match application.properties, the 8 error codes all exist in OpenAiErrorResponse, and the documented endpoints match RestOpenAiAdapter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 47 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
src/main/java/ai/labs/eddi/integrations/openai/OpenAiOutputRenderer.java:203
- Quick reply values are wrapped in backticks without escaping. If a quick reply label contains a backtick, the rendered Markdown breaks and may hide or mangle the suggested reply list.
if (values.isEmpty()) {
return null;
}
return "_Suggested replies:_ " + String.join(" · ", values.stream().map(v -> "`" + v + "`").toList());
}
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:279
- resolveOrFail() maps all UnknownModelException cases to 404 model_not_found, including when the client omitted the required
modelfield (AgentModelResolver throws "No model was specified."). A missing required field is a malformed request and should be a 400 so clients don’t mis-handle it as “unknown model”.
try {
return modelResolver.resolve(modelId);
} catch (AgentModelResolver.UnknownModelException e) {
throw OpenAiApiException.notFound(OpenAiErrorResponse.CODE_MODEL_NOT_FOUND, e.getMessage());
} catch (AgentModelResolver.AmbiguousModelException e) {
src/main/java/ai/labs/eddi/integrations/openai/model/ContentPart.java:53
- ContentPart.isImageUrl()/isInputAudio() treat empty strings as valid payloads (they only check for null). That causes the mapper to build “attachments” with blank URLs / data, which are guaranteed to fail downstream and makes the adapter less tolerant of real-world clients that sometimes send empty fields.
public boolean isImageUrl() {
return TYPE_IMAGE_URL.equals(type) && imageUrl != null && imageUrl.url() != null;
}
public boolean isInputAudio() {
src/main/java/ai/labs/eddi/integrations/openai/OpenAiAuthFilter.java:195
- The constantTimeEquals() Javadoc claims it avoids leaking the length relationship through timing, but MessageDigest.isEqual runs in time proportional to min(a.length, b.length). Either implement a length-constant compare or adjust the comment so it doesn’t over-promise what the code provides.
/**
* Compare secrets without leaking their length relationship through timing.
* {@code String.equals} short-circuits on the first differing character.
*/
private static boolean constantTimeEquals(String presented, String expected) {
src/main/java/ai/labs/eddi/integrations/openai/RestOpenAiAdapter.java:162
- When
stateless:trueis set in the request body, the adapter correctly forces the resolved model to stateless, but the wiremodelfield (JSON response + SSE frames) still echoesrequest.model()without the:statelesssuffix. That makes the response describe a different model than the one actually executed, and it makes the body field behave differently from the suffix route.
if (request.isStreaming()) {
return streamingResponse(turn, completionId, request.model(), created, request.includeUsage());
}
var outcome = bridge.say(turn, model, userId, headers, request);
src/main/java/ai/labs/eddi/datastore/postgres/PostgresUserConversationStore.java:145
- PR description says “no existing Java file is modified”, but this PR updates PostgresUserConversationStore (log sanitization) as part of the adapter work. Please update the PR description (or the claim) so reviewers don’t miss the non-additive change.
// Sanitized: the intent embeds a caller-supplied chat key (the OpenAI
// adapter builds channel:openai:<agentId>:<chatKey> from a request
// header), so a newline in it could forge log entries.
LOGGER.error("Failed to delete user conversation intent=" + sanitize(intent), e);
}
aisabella-ai
approved these changes
Jul 29, 2026
ginccc
added a commit
that referenced
this pull request
Jul 29, 2026
#614 merged to main and left this PR CONFLICTING — which matters beyond tidiness, because a conflicting PR has no computable merge ref and so cannot run CI at all. One conflict, docs/changelog.md: both sides added a top entry. Both kept, nothing dropped. Verified past the textual resolution, since a clean auto-merge has silently broken compilation twice in this stack when main changed a constructor this branch's tests were written against: the merged tree compiles, and 208 tests across both sides of the merge pass (WorkflowTraversal and MemoryItemConverter from this branch, the OpenAI adapter surface from #614).
ginccc
added a commit
that referenced
this pull request
Jul 29, 2026
#614 merged and left this PR CONFLICTING, which blocks CI entirely — a conflicting PR has no computable merge ref, so no workflow can run on it. One conflict, docs/changelog.md: both sides added top entries. Both kept. Verified past the resolution: the merged tree compiles and 888 tests pass across both sides — this branch's schedule/Dream/shutdown/group work plus #614's OpenAI adapter surface.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
/v1adapter presenting deployed EDDI agents as OpenAI "models", so Open WebUI, the PythonopenaiSDK, LangChain and LiteLLM can drive an EDDI conversation without knowing about EDDI.New package
integrations/openai/, parallel tointegrations/slack/. Purely additive outside the adapter:application.propertiesgains lines only,AGENTS.mdgains one roadmap row, and no existing Java file is modified.Full guide:
docs/open-webui-integration.md. Design record and research:planning/openai-api-adapter-plan.md.What it does
GET /v1/models,GET /v1/models/{id},POST /v1/chat/completions(sync JSON + SSE streaming)X-OpenWebUI-Chat-Id— each Open WebUI chat window gets its own EDDI conversation, memory and allimage_url,file(inline PDFs/documents) andinput_audio, all mapped toattachment_Ncontext so EDDI's existing forwarder does the real work — capability gating, byte caps, PDF text extraction, SSRF-guarded fetching200assistant messages, never as HTTP errors:statelessmodel suffix or astatelessbody fieldIConversationServiceDisabled by default (
eddi.openai-compat.enabled=false).Four earlier plan drafts rested on false premises
Each was verified against source before implementing; the corrections shaped the design. Recorded in the changelog with evidence so they don't get re-litigated:
Acceptheaderopenai-pythonhardcodesAccept: application/jsonand never varies it bystream; Open WebUI sends noAcceptheader at all. → one method, dispatching on thestreambody field.chat_id→ theuserfieldpayload['user']is set only for pipeline models and is an object, not a chat id. → key onX-OpenWebUI-Chat-Id(#15813).IRestAgentAdministrationfor/v1/models@RolesAllowed({"eddi-admin","eddi-editor"}), enforced by a CDI interceptor → 403 for every ordinary caller. → read the stores directly. REST facades are the authorization boundary./,/*→authenticatedcaptures/v1/*, so ansk-…bearer is rejected by OIDC before the adapter runs. → explicit/v1/*permission entry + two auth modes + startup guard.Two components from the plan were cut:
UtilityAgentProvisioner(unauthenticated caller creating and deploying an agent, consuming tenant quota — replaced by:stateless), and the new-chat heuristic (unreliable, destructive, and self-contradictory with the plan's own recommended Open WebUI filter).Security
authorization.enabled=true, naming the three fixestrust-user-headersis a documented delegation, flag-gated: a leaked key permits impersonation. Called out in the guide as a warning, not a footnote.Testing
143 unit tests, 0 failures;
clean compileand Checkstyle green.Mutation-checked rather than assumed — reverting each of these makes its tests fail: the model-ambiguity guard, the identity refusal, the HITL sentinel distinction, the stateless config gate, audio MIME mapping, filename-over-generic-type, token/snapshot reconciliation, and null-message handling.
A self-review pass after the feature was complete found six defects, all fixed and covered (see
5b8c471). The most serious: a semaphore permit leak on the streaming path that would have made the adapter return429permanently after enough client disconnects, with nothing in the logs to explain it.Known gap — please review with this in mind
No test exercises the endpoint over real HTTP. The load-bearing assumption — that
@Produces({APPLICATION_JSON, SERVER_SENT_EVENTS})+StreamingOutputyieldstext/event-streamwhenstream:true— is verified by reasoning only. Socket-binding tests can't run in the dev sandbox, so this needs a@QuarkusTestin CI and is the first thing worth covering.Other stated gaps:
usage/token counts omitted (not faked as zeros); quick replies andinputFieldoutputs dropped;tool_callsnever returned (would double-execute);/v1/embeddingsnot implemented.Follow-ups, deliberately not in this PR
SlackHitlSupport.extractSlackResponseTextduplicates the newer sharedConversationOutputExtractor, which handles more formats — worth deduplicating:statelessshould replay client history to become genuinely OpenAI-compatible for history-managing clientsSummary by CodeRabbit
/v1adapter forGET /v1/modelsandPOST /v1/chat/completions(streaming SSE and non-streaming), exposing deployed agents as models with optional stateless variants, multimodal attachments, and HITL-aware pause handling.