Skip to content

Fix/mcp endpoint bugs - #515

Merged
ginccc merged 9 commits into
mainfrom
fix/mcp-endpoint-bugs
Jun 3, 2026
Merged

Fix/mcp endpoint bugs#515
ginccc merged 9 commits into
mainfrom
fix/mcp-endpoint-bugs

Conversation

@ginccc

@ginccc ginccc commented Jun 3, 2026

Copy link
Copy Markdown
Member

This pull request delivers a version bump from 6.0.2 to 6.1.0, reflecting a comprehensive set of MCP endpoint bug fixes, dependency updates, and workflow/documentation improvements. Key changes include resolving several MCP bugs (notably in chat_managed and group discussion error handling), enforcing stricter resource deletion semantics, and updating all deployment and documentation references to the new version.

MCP Endpoint Bug Fixes and Hardening:

  • Fixed chat_managed always returning "Internal error" by adding the missing @Blocking annotation, refactoring conversation startup to avoid JAX-RS exception wrapping, and improving stale conversation cleanup.
  • Group discussions now show a clear error message when the LLM fails, rather than empty transcript entries.
  • delete_agent_trigger now throws ResourceNotFoundException for nonexistent intents in both Mongo and Postgres stores, and database errors are no longer silently swallowed.
  • Fixed NPE in read_conversation_log by guarding against null logSize.
  • RestVersionInfo now correctly overrides getCurrentResourceId() to prevent failures in group operations with version=0.

Version Bump and Artifact Updates:

  • Updated project version from 6.0.2 to 6.1.0 in all deployment artifacts and documentation, including pom.xml, Dockerfile, Helm chart, Kubernetes manifests, and agent ZIP bundles. [1] [2] [3] [4] [5] F4bf5d55L5R5, [6]

Workflow and Documentation Consistency:

  • Updated Red Hat certification workflow and related documentation to reference version 6.1.0 and standardized YAML formatting for consistency. [1] [2] [3] [4]

Documentation Table Formatting:

  • Improved markdown table formatting for readability and alignment in docs/redhat-openshift.md. [1] [2] [3] [4] [5]

Changelog and Release Notes:

  • Added detailed changelog entries describing all MCP bug fixes, follow-ups, and code review observations for traceability.

Summary by CodeRabbit

  • New Features

    • Improved conversation lifecycle handling and field-level filtering for conversation outputs.
  • Bug Fixes

    • Resolved managed-conversation errors and stale-mapping issues.
    • Restored transcript content when agents fail and improved log/version null-safety.
    • Fixed multiple endpoint/audit issues (deserialization, filtering, delete semantics, metadata handling).
  • Documentation

    • Bumped product version to 6.1.0 across docs, charts, manifests, and examples.
  • Tests

    • Added extensive regression tests covering conversation lifecycle, log reading, metadata detection, triggers, and filtering.

ginccc added 6 commits June 2, 2026 22:58
BUG-1: read_resource langchain empty {} - add @JsonProperty to record
BUG-2: group discussion raw actions - filter metadata-only outputs
BUG-3: list_conversations agent filter - use getAgentResource()
BUG-4: read_conversation_log NPE - null guard for logSize
BUG-5: delete_agent_trigger 200 for missing - throw ResourceNotFoundException
BUG-6: chat_managed stale trigger - validate trigger before reuse
BUG-7: version=0 resolution - delegate getCurrentResourceId in RestVersionInfo
BUG-8: returningFields ignored - add post-filter in MCP layer

19 regression tests across 5 new test files.
ISSUE-1: PostgresAgentTriggerStore.deleteAgentTrigger() now throws
ResourceStoreException on SQLException instead of silently swallowing.

ISSUE-2: BUG-8 field filter creates filtered copies instead of mutating
the live ConversationOutput map via removeIf (prevents cache corruption).

ISSUE-3: BUG-2 metadata detection replaced fragile positive-list
(3 hardcoded keys) with resilient absence-of-output check
(startsWith output/reply). Resilient to new pipeline metadata keys.

OBS-1: BUG-6 trigger validation narrowed from catch(Exception) to
catch(ResourceNotFoundException) — transient DB errors now propagate
instead of falsely deleting UserConversation state.
Variable was renamed agentResourceId → conversationResourceId in BUG-3
but log message text was not updated to match.
…ntent

- chatManaged: add missing @Blocking annotation (event-loop blocking)
- chatManaged: use conversationService directly instead of JAX-RS layer
- chatManaged: handle stale UserConversation when conversation deleted
- GroupConversationService: surface ERROR state as transcript content
  when extractResponse returns null (BUG-2 follow-up)
… bugs

McpConversationToolsTest (6 new tests, 42 total):
- chatManaged_happyPath: full flow — create conversation + send message
- chatManaged_existingConversation_reusesIt: no new conversation created
- chatManaged_staleConversation_recreatesFresh: deleted conv → cleanup + recreate
- chatManaged_triggerDeleted_returnsError: sneakyThrow ResourceNotFoundException
- chatManaged_conversationCreationFails_returnsError: AgentNotReadyException
- (existing) chatManaged_noTriggerConfigured_returnsError: null trigger

GroupConversationServiceTest (1 new test, 28 total):
- agentErrorState_transcriptContainsErrorMessage: BUG-2 regression test
  — agent returns ERROR state with no output keys
  — verifies transcript entry has error message, not null
@ginccc
ginccc requested a review from rolandpickl as a code owner June 3, 2026 12:23
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ginccc, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 25 minutes and 7 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8daf9c59-125f-4650-b98b-cf159728cd47

📥 Commits

Reviewing files that changed from the base of the PR and between 81f3e02 and 9589dec.

📒 Files selected for processing (1)
  • src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java
📝 Walkthrough

Walkthrough

This release (6.1.0) bumps versions across artifacts, fixes MCP managed-conversation lifecycle and output filtering, makes conversation log handling null-safe, tightens agent-trigger delete semantics and agent filtering, adds a Jackson mapping annotation, and adds regression tests and documentation updates.

Changes

Release 6.1.0 — Bug Fixes & Feature Enhancements

Layer / File(s) Summary
Version Bump & Infrastructure Updates
pom.xml, helm/eddi/Chart.yaml, k8s/base/eddi-deployment.yaml, k8s/quickstart.yaml, src/main/docker/Dockerfile, src/main/resources/application.properties, src/main/resources/initial-agents/available_agents.txt, .github/workflows/redhat-certify.yml, docs/changelog.md, docs/redhat-openshift.md, README.md
Version bumped 6.0.2 → 6.1.0 across Maven, Helm, Kubernetes manifests, Docker build args, application properties, and agent archives. Workflow dispatch inputs normalized (quote/array formatting); Red Hat certification docs reformatted and examples updated.
Conversation Log Null-Safety
src/main/java/ai/labs/eddi/engine/internal/ConversationService.java, src/test/java/ai/labs/eddi/engine/internal/ConversationServiceReadLogTest.java
readConversationLog treats null logSize as -1 to avoid NPE-like behavior. Tests verify null/zero/positive logSize behavior.
Group Discussion Transcript Output Handling
src/main/java/ai/labs/eddi/engine/internal/GroupConversationService.java, src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtractResponseTest.java, src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceTest.java
extractResponse() treats metadata-only outputs (no output*/reply keys) as null so metadata isn't serialized; when agent snapshot is ERROR, an explicit failure message is recorded. Tests cover edge cases and transcript behavior.
MCP Conversation Tools — Field Filtering & Lifecycle
src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java, src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java
read_conversation applies per-field filtering to conversation outputs; chat_managed marked @Blocking. getOrCreateManagedConversation revalidates triggers, removes stale mappings, recreates conversations when needed, and calls conversationService.startConversation(...) directly. Tests exercise happy path, reuse, stale/ended mappings, trigger deletion, and creation failures.
Conversation Filtering & Trigger Delete Semantics
src/main/java/ai/labs/eddi/engine/memory/rest/RestConversationStore.java, src/main/java/ai/labs/eddi/engine/triggermanagement/IAgentTriggerStore.java, src/main/java/ai/labs/eddi/engine/triggermanagement/mongo/AgentTriggerStore.java, src/main/java/ai/labs/eddi/engine/triggermanagement/postgres/PostgresAgentTriggerStore.java, src/main/java/ai/labs/eddi/engine/triggermanagement/rest/RestAgentTriggerStore.java, src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java, src/test/java/ai/labs/eddi/engine/triggermanagement/rest/RestAgentTriggerStoreTest.java
RestConversationStore now derives agentResourceId separately for correct agentId/version filtering. Trigger-store delete APIs now declare/throw ResourceNotFoundException; Mongo/Postgres implementations validate deletion and throw when not found; REST layer rethrows exceptions accordingly. Tests validate filtering and delete error behavior.
API & Data Model Fixes
src/main/java/ai/labs/eddi/configs/rest/RestVersionInfo.java, src/main/java/ai/labs/eddi/modules/llm/model/LlmConfiguration.java, src/test/java/ai/labs/eddi/configs/rest/RestVersionInfoTest.java
RestVersionInfo adds getCurrentResourceId() delegation to the resource store. LlmConfiguration.tasks now explicitly annotated @JsonProperty("tasks") for reliable Jackson binding. Tests verify delegation and exception propagation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • rolandpickl

🐰 I hopped the version fence with glee,
Six-one-oh sings — updated we be,
MCP tends maps and trims outputs neat,
Logs behave, triggers meet their beat,
Tests hop in — release looks complete!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.33% which is insufficient. The required threshold is 80.00%. 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 'Fix/mcp endpoint bugs' directly corresponds to the main objectives: fixing multiple MCP endpoint bugs (delete_agent_trigger, read_conversation_log, chat_managed, and group discussion hardening).
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mcp-endpoint-bugs

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 and usage tips.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Comment thread src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR bumps EDDI to 6.1.0 and hardens/fixes multiple MCP endpoints (notably chat_managed, group discussion transcript error handling, agent trigger deletion semantics, conversation filtering/log reading, and version=0 handling), with added regression tests and updated deployment/documentation version references.

Changes:

  • Fixed and hardened MCP behavior in chat_managed, read_conversation field filtering, and group discussion transcript/error handling.
  • Enforced stricter deletion semantics for agent triggers (Mongo + Postgres) by throwing ResourceNotFoundException on no-op deletes, and updated REST handling.
  • Bumped version references to 6.1.0 across build/deploy artifacts and documentation; added/updated bundled initial agent ZIP.

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java Fixes chat_managed blocking/creation flow and adds output field filtering for read_conversation.
src/main/java/ai/labs/eddi/engine/internal/GroupConversationService.java Prevents metadata-only outputs from appearing as agent replies; adds explicit error content when agent is in ERROR state.
src/main/java/ai/labs/eddi/engine/internal/ConversationService.java Guards readConversationLog against null logSize unboxing NPE.
src/main/java/ai/labs/eddi/engine/memory/rest/RestConversationStore.java Corrects agentId/agentVersion filtering to use agentResource instead of conversation resource URI.
src/main/java/ai/labs/eddi/engine/triggermanagement/IAgentTriggerStore.java Updates delete contract to include ResourceNotFoundException.
src/main/java/ai/labs/eddi/engine/triggermanagement/mongo/AgentTriggerStore.java Throws ResourceNotFoundException when delete count is zero (Mongo).
src/main/java/ai/labs/eddi/datastore/postgres/PostgresAgentTriggerStore.java Throws ResourceNotFoundException on zero-row delete and rethrows DB failures as ResourceStoreException (Postgres).
src/main/java/ai/labs/eddi/engine/triggermanagement/rest/RestAgentTriggerStore.java Propagates ResourceNotFoundException/ResourceStoreException via sneakyThrow for REST mapping.
src/main/java/ai/labs/eddi/configs/rest/RestVersionInfo.java Implements getCurrentResourceId() delegation to underlying store to support version=0 flows.
src/main/java/ai/labs/eddi/modules/llm/model/LlmConfiguration.java Adds @JsonProperty("tasks") to record component to ensure reliable deserialization.
src/test/java/.../* (new/updated tests) Adds regression coverage for the above MCP/REST/store issues.
pom.xml, src/main/resources/application.properties, Dockerfile, helm/*, k8s/*, docs/*, .github/workflows/redhat-certify.yml, initial-agents/* Version bump to 6.1.0 and associated artifact/doc updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/redhat-certify.yml
Comment thread docs/changelog.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (1)
src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java (1)

91-137: ⚡ Quick win

Add one agentVersion regression case here too.

The production fix changed both agentId and agentVersion filtering to use getAgentResource(), but these tests only lock down the id branch. A version-match/version-mismatch case would catch regressions in lines 132-134 as well.

🤖 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/engine/memory/rest/RestConversationStoreFilterTest.java`
around lines 91 - 137, Add two additional assertions/tests covering agentVersion
behavior: duplicate the existing agentId tests but set the descriptor and
snapshot to use the same AGENT_ID while varying agentVersion—one test where the
descriptor/snapshot version equals the requested agentVersion (expect result
size 1) and one where it differs (expect result size 0). Use the same helpers
used in these tests (createDescriptor(CONVERSATION_ID, AGENT_ID, <version>),
createSnapshot(AGENT_ID, <version>), mock
documentDescriptorStore.readDescriptor(AGENT_ID, <version>) to return a
DocumentDescriptor) and call
restConversationStore.readConversationDescriptors(..., AGENT_ID, <agentVersion>,
... ) to assert matching and non-matching behaviors; this will cover the
agentVersion branch that relies on getAgentResource().
🤖 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 @.github/workflows/redhat-certify.yml:
- Around line 7-9: Update the workflow input default to match the description by
changing the "default" value from "6.0.2" to "6.1.0"; modify the YAML key named
default (paired with the description: "EDDI version (e.g. 6.1.0)") so the
default version string equals "6.1.0" to avoid certifying the wrong release when
using defaults.

In `@docs/changelog.md`:
- Around line 7-99: Add the missing "Design decisions" and "What's in progress /
What's next" subsections under the two new version headings ("## 📦 Version Bump
6.0.2 → 6.1.0 (2026-06-03)" and "## 🐛 MCP Endpoint Bug Fixes — 8 Bugs Resolved
(2026-06-02)") in docs/changelog.md; under each "Design decisions" briefly list
the key architectural/behavioral choices made (e.g., use of `@Blocking` and switch
from restAgentEngine.startConversationWithContext to
conversationService.startConversation, stale-conversation cleanup in
McpConversationTools.java, null-handling in GroupConversationService.java,
LlmConfiguration `@JsonProperty` change, etc.), and under "What's in progress /
What's next" note any follow-ups, regression test coverage, remaining PRs or
monitoring steps (e.g., verify trigger invalidation paths, redhat certification
workflow, monitor group discussion ERROR cases) so the entries comply with the
repo changelog standard.

In `@docs/redhat-openshift.md`:
- Line 44: The table entry claiming the base image is "pinned by SHA256 digest"
is inconsistent with the example which only shows the tag
`registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24`; update the doc to
either include the actual digest form `@sha256:...` alongside that image
everywhere the base image is shown (including the later example around the same
section) or remove/rewrite the "pinned by SHA256 digest" wording so it does not
claim digest pinning; ensure all occurrences of
`registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24` are made consistent
with your choice.

In `@k8s/base/eddi-deployment.yaml`:
- Line 8: The manifest's app.kubernetes.io/version label ("6.1.0") does not
match the container image tag (labsai/eddi:6); update them to be consistent by
setting the container image in the Deployment spec (the container image field
where labsai/eddi:6 is used) to the exact release tag 6.1.0 (or alternatively
change the app.kubernetes.io/version label to "6" if you intentionally want the
image stream tag); ensure both app.kubernetes.io/version and the image tag match
(e.g., "6.1.0") to prevent silent drift.

In `@k8s/quickstart.yaml`:
- Line 172: The deployment metadata label app.kubernetes.io/version ("6.1.0")
does not match the container image tag (labsai/eddi:6), risking mismatched
patches; update the Deployment spec to use the exact image tag that matches the
metadata (e.g., change the image from labsai/eddi:6 to labsai/eddi:6.1.0) or
alternatively set app.kubernetes.io/version to "6" so they remain
consistent—locate the label declaration app.kubernetes.io/version and the
container image field (labsai/eddi:6) in the Deployment manifest and make them
identical across the YAML.

In `@src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java`:
- Around line 553-564: The code currently catches all exceptions from
restAgentEngine.getConversationState(...) and treats them as "not found",
causing valid mappings to be deleted on transient failures; change the error
handling so only the explicit "conversation not found" case triggers
deletion/recreation. Replace the broad catch(Exception) around
restAgentEngine.getConversationState with a targeted check: if
getConversationState returns ENDED delete via
userConversationStore.deleteUserConversation(intent, userId), and if it throws
the specific not-found exception (e.g., ConversationNotFoundException or the
concrete exception type thrown by restAgentEngine.getConversationState) treat
that as stale and delete/recreate; for any other exception rethrow (or
propagate) so transient DB/REST failures do not drop the mapping. Ensure
references to restAgentEngine.getConversationState, ConversationState.ENDED, and
userConversationStore.deleteUserConversation are updated accordingly.
- Around line 235-247: The current filtering rewrites every map from
snapshot.getConversationOutputs() even when the caller requested the entire
section (e.g., returningFields includes "conversationOutputs"), stripping nested
keys; update the logic in McpConversationTools where trimmedFields and
filteredOutputs are computed to first detect if trimmedFields contains
"conversationOutputs" (or the section name requested) and, if so, skip the
per-entry map filtering and leave snapshot.getConversationOutputs() unchanged;
otherwise keep the existing map filtering that matches keys via equals or
startsWith. Ensure you reference snapshot.getConversationOutputs(),
trimmedFields, and the mapping that builds filteredOutputs when making the
change.

In `@src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java`:
- Around line 611-700: Add a regression test in McpConversationToolsTest that
simulates a transient failure from RestAgentEngine.getConversationState (throw a
generic RuntimeException or similar) when an existing
ai.labs.eddi.engine.triggermanagement.model.UserConversation exists; call
tools.chatManaged(...) and assert it returns an error string, then verify that
userConversationStore.deleteUserConversation(...) was NOT called and
conversationService.startConversation(...) was NOT called (so the stale mapping
is preserved and no new conversation is created). Use the same setup style as
the existing tests (mocking userConversationStore.readUserConversation,
AgentTriggerStore.readAgentTrigger, and jsonSerialization) and reference
RestAgentEngine.getConversationState,
userConversationStore.deleteUserConversation, and
conversationService.startConversation in your assertions.

---

Nitpick comments:
In
`@src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java`:
- Around line 91-137: Add two additional assertions/tests covering agentVersion
behavior: duplicate the existing agentId tests but set the descriptor and
snapshot to use the same AGENT_ID while varying agentVersion—one test where the
descriptor/snapshot version equals the requested agentVersion (expect result
size 1) and one where it differs (expect result size 0). Use the same helpers
used in these tests (createDescriptor(CONVERSATION_ID, AGENT_ID, <version>),
createSnapshot(AGENT_ID, <version>), mock
documentDescriptorStore.readDescriptor(AGENT_ID, <version>) to return a
DocumentDescriptor) and call
restConversationStore.readConversationDescriptors(..., AGENT_ID, <agentVersion>,
... ) to assert matching and non-matching behaviors; this will cover the
agentVersion branch that relies on getAgentResource().
🪄 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

Run ID: b7166c79-8ee7-4ce0-84dd-36dad7bbd845

📥 Commits

Reviewing files that changed from the base of the PR and between 4f2d134 and 036faa3.

⛔ Files ignored due to path filters (1)
  • src/main/resources/initial-agents/Agent+Father-6.1.0.zip is excluded by !**/*.zip
📒 Files selected for processing (27)
  • .github/workflows/redhat-certify.yml
  • docs/changelog.md
  • docs/redhat-openshift.md
  • helm/eddi/Chart.yaml
  • k8s/base/eddi-deployment.yaml
  • k8s/quickstart.yaml
  • pom.xml
  • src/main/docker/Dockerfile
  • src/main/java/ai/labs/eddi/configs/rest/RestVersionInfo.java
  • src/main/java/ai/labs/eddi/datastore/postgres/PostgresAgentTriggerStore.java
  • src/main/java/ai/labs/eddi/engine/internal/ConversationService.java
  • src/main/java/ai/labs/eddi/engine/internal/GroupConversationService.java
  • src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java
  • src/main/java/ai/labs/eddi/engine/memory/rest/RestConversationStore.java
  • src/main/java/ai/labs/eddi/engine/triggermanagement/IAgentTriggerStore.java
  • src/main/java/ai/labs/eddi/engine/triggermanagement/mongo/AgentTriggerStore.java
  • src/main/java/ai/labs/eddi/engine/triggermanagement/rest/RestAgentTriggerStore.java
  • src/main/java/ai/labs/eddi/modules/llm/model/LlmConfiguration.java
  • src/main/resources/application.properties
  • src/main/resources/initial-agents/available_agents.txt
  • src/test/java/ai/labs/eddi/configs/rest/RestVersionInfoTest.java
  • src/test/java/ai/labs/eddi/engine/internal/ConversationServiceReadLogTest.java
  • src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceExtractResponseTest.java
  • src/test/java/ai/labs/eddi/engine/internal/GroupConversationServiceTest.java
  • src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java
  • src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java
  • src/test/java/ai/labs/eddi/engine/triggermanagement/rest/RestAgentTriggerStoreTest.java

Comment thread .github/workflows/redhat-certify.yml Outdated
Comment thread docs/changelog.md
Comment thread docs/redhat-openshift.md
Comment thread k8s/base/eddi-deployment.yaml
Comment thread k8s/quickstart.yaml
Comment thread src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java Outdated
Comment thread src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java
Comment thread src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
docs/redhat-openshift.md (1)

44-44: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Digest pinning claim and examples remain inconsistent.

The documentation still claims the base image is "pinned by SHA256 digest" (lines 44, 154), but the examples only show the tag :1.24 without any @sha256:... suffix (lines 44, 153). This inconsistency was previously flagged and remains unresolved.

Please either:

  1. Include the actual digest in all base image references (e.g., registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24@sha256:...), or
  2. Remove/reword the digest pinning claims if the examples won't show the full digest form.

As per coding guidelines, the Dockerfile must use digest pinning, so the documentation should accurately reflect this practice in its examples.

Also applies to: 153-154

🤖 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 `@docs/redhat-openshift.md` at line 44, The doc text claims the base image is
"pinned by SHA256 digest" but the examples show only the tag; update all
occurrences of the base image string
registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24 (both example
references) to the digest-pinned form
registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24@sha256:<actual-digest>
and ensure the surrounding sentence explicitly states it is pinned by SHA256
digest; if you cannot obtain the digest, remove/reword the "pinned by SHA256
digest" phrase so examples and claim are consistent (prefer adding the actual
digest to satisfy the Dockerfile/digest-pinning guideline).
🧹 Nitpick comments (1)
src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java (1)

145-150: ⚡ Quick win

Harden version-filter regression by decoupling snapshot version from agentResource version.

These tests currently keep snapshot and agentResource versions equal, so an implementation that incorrectly filters by snapshot version could still pass. Make them intentionally different to prove filtering is sourced from getAgentResource().

Proposed test hardening
@@
-        when(conversationMemoryStore.loadConversationMemorySnapshot(CONVERSATION_ID))
-                .thenReturn(createSnapshot(AGENT_ID, 1));
+        // Intentionally different from descriptor agentResource version to ensure
+        // filtering is based on agentResource, not snapshot metadata.
+        when(conversationMemoryStore.loadConversationMemorySnapshot(CONVERSATION_ID))
+                .thenReturn(createSnapshot(AGENT_ID, 99));
@@
-        when(conversationMemoryStore.loadConversationMemorySnapshot(CONVERSATION_ID))
-                .thenReturn(createSnapshot(AGENT_ID, 1));
+        // Intentionally different from descriptor agentResource version to ensure
+        // filtering is based on agentResource, not snapshot metadata.
+        when(conversationMemoryStore.loadConversationMemorySnapshot(CONVERSATION_ID))
+                .thenReturn(createSnapshot(AGENT_ID, 99));

Also applies to: 171-177

🤖 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/engine/memory/rest/RestConversationStoreFilterTest.java`
around lines 145 - 150, The test currently uses matching versions for the agent
resource and the snapshot (createDescriptor(..., 1) and createSnapshot(..., 1)),
which can mask a bug that filters by snapshot version; update the test to
decouple these by creating the agent descriptor with one version and returning a
snapshot with a different version (e.g., createDescriptor(CONVERSATION_ID,
AGENT_ID, 2) but
when(conversationMemoryStore.loadConversationMemorySnapshot(...)).thenReturn(createSnapshot(AGENT_ID,
1))) so the filter must rely on getAgentResource() version rather than the
snapshot; apply the same change to the other test case that mirrors lines
171-177.
🤖 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.

Duplicate comments:
In `@docs/redhat-openshift.md`:
- Line 44: The doc text claims the base image is "pinned by SHA256 digest" but
the examples show only the tag; update all occurrences of the base image string
registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24 (both example
references) to the digest-pinned form
registry.access.redhat.com/ubi9/openjdk-25-runtime:1.24@sha256:<actual-digest>
and ensure the surrounding sentence explicitly states it is pinned by SHA256
digest; if you cannot obtain the digest, remove/reword the "pinned by SHA256
digest" phrase so examples and claim are consistent (prefer adding the actual
digest to satisfy the Dockerfile/digest-pinning guideline).

---

Nitpick comments:
In
`@src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java`:
- Around line 145-150: The test currently uses matching versions for the agent
resource and the snapshot (createDescriptor(..., 1) and createSnapshot(..., 1)),
which can mask a bug that filters by snapshot version; update the test to
decouple these by creating the agent descriptor with one version and returning a
snapshot with a different version (e.g., createDescriptor(CONVERSATION_ID,
AGENT_ID, 2) but
when(conversationMemoryStore.loadConversationMemorySnapshot(...)).thenReturn(createSnapshot(AGENT_ID,
1))) so the filter must rely on getAgentResource() version rather than the
snapshot; apply the same change to the other test case that mirrors lines
171-177.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ad6cecff-5c9e-4503-8176-1214baffed58

📥 Commits

Reviewing files that changed from the base of the PR and between 036faa3 and 81f3e02.

📒 Files selected for processing (7)
  • .github/workflows/redhat-certify.yml
  • README.md
  • docs/changelog.md
  • docs/redhat-openshift.md
  • src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java
  • src/test/java/ai/labs/eddi/engine/mcp/McpConversationToolsTest.java
  • src/test/java/ai/labs/eddi/engine/memory/rest/RestConversationStoreFilterTest.java
✅ Files skipped from review due to trivial changes (3)
  • README.md
  • .github/workflows/redhat-certify.yml
  • docs/changelog.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/ai/labs/eddi/engine/mcp/McpConversationTools.java

@ginccc
ginccc merged commit 5356e79 into main Jun 3, 2026
23 checks passed
@ginccc
ginccc deleted the fix/mcp-endpoint-bugs branch June 3, 2026 16:04
@coderabbitai coderabbitai Bot mentioned this pull request Jun 23, 2026
13 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Jul 15, 2026
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants